fix(shared): accept namespaced ext: tag keys in relay filters #1

Merged

laoc merged 1 commit from feature/ext-tag-filter into main

2026-07-30 20:55:25 +00:00

laoc commented

2026-07-30 19:54:56 +00:00

Owner

The problem

swentel/nostr-php validates every filter tag name against /^#[a-z_-]+$/i (Filter::validateTagName). The colons in the NIP-AMB extension grammar do not pass it:

#l                                     ACCEPTED
#L                                     ACCEPTED
#d                                     ACCEPTED
#start_after                           ACCEPTED
#ext:ekw:schoolType:id                 REJECTED  RuntimeException: Invalid tag provided
#ext:org.edufeed.ekw.konfi:themen:id   REJECTED  RuntimeException: Invalid tag provided

NostrFetcher::fetch() passes $filterParams['tags'] straight to setTag(), so every ext: facet filter throws before the REQ is built — and ext: tag-key filtering is the primary filter mechanism for the Materialpool plugin.

A version bump does not fix this. The regex is byte-identical in 1.9.2 (vendored), 1.9.3, 1.9.4 and upstream main.

The fix

validateTagName is private, so it cannot be overridden. setTag is public and is its only caller inside its own body, so ExtendedFilter overrides setTag/setTags and inherits everything else — dynamic property storage, toArray(), and therefore the generated REQ. The wire format stays the parent's, which is what makes the override safe.

The replacement pattern is /^#[a-zA-Z0-9_.:-]+$/. The leading # stays mandatory: a key without it is silently skipped by the relay's filter parser, so the query runs unfiltered — a typo would widen the result set instead of erroring.

Single-letter tags are unaffected; #l, #L and #d already passed upstream and still do.

Evidence

Live against wss://amb-relay.edufeed.org, through NostrFetcher itself:

#ext:ekw:gradeLevel:id = <real value>    ->    3
#ext:ekw:gradeLevel:id = <bogus value>   ->    0   (twice)
#l = ekw                                 ->   33   (COUNT agrees: 33)
kinds only, no tag filter                ->  100   (page limit)

The bogus-value control is part of the evidence, not decoration: if a bogus value returned the same count as a real one, the filter would be a no-op and the number meaningless.

phpunit (shared/, --exclude-group integration)   OK (15 tests, 22 assertions)

Verified non-vacuous: delegating setTag back to parent::setTag fails 3 of the 15.

One test asserts that the upstream Filter still rejects #ext:.... If that test ever fails, upstream has widened its regex and ExtendedFilter can be deleted.

Follow-ups, not in this PR

Upstream PR to widen the regex in nostrver-se/nostr-php, so this class can eventually go away.
swentel/nostr-php is pinned >=1.5 <1.9.3 and 1.9.4 is out. Upgrading pulls in the replacement for the abandoned leigh/chacha20 and a request timeout property, but raises the floor to PHP 8.2 — CLAUDE.md documents the stack as PHP 8.1+, so that is a redistributability decision, not a mechanical bump.

## The problem `swentel/nostr-php` validates every filter tag name against `/^#[a-z_-]+$/i` (`Filter::validateTagName`). The colons in the NIP-AMB extension grammar do not pass it: ``` #l ACCEPTED #L ACCEPTED #d ACCEPTED #start_after ACCEPTED #ext:ekw:schoolType:id REJECTED RuntimeException: Invalid tag provided #ext:org.edufeed.ekw.konfi:themen:id REJECTED RuntimeException: Invalid tag provided ``` `NostrFetcher::fetch()` passes `$filterParams['tags']` straight to `setTag()`, so **every `ext:` facet filter throws before the REQ is built** — and `ext:` tag-key filtering is the primary filter mechanism for the Materialpool plugin. **A version bump does not fix this.** The regex is byte-identical in 1.9.2 (vendored), 1.9.3, 1.9.4 and upstream `main`. ## The fix `validateTagName` is `private`, so it cannot be overridden. `setTag` is `public` and is its only caller inside its own body, so `ExtendedFilter` overrides `setTag`/`setTags` and inherits everything else — dynamic property storage, `toArray()`, and therefore the generated REQ. **The wire format stays the parent's**, which is what makes the override safe. The replacement pattern is `/^#[a-zA-Z0-9_.:-]+$/`. The leading `#` stays mandatory: a key without it is *silently skipped* by the relay's filter parser, so the query runs unfiltered — a typo would widen the result set instead of erroring. Single-letter tags are unaffected; `#l`, `#L` and `#d` already passed upstream and still do. ## Evidence Live against `wss://amb-relay.edufeed.org`, through `NostrFetcher` itself: ``` #ext:ekw:gradeLevel:id = <real value> -> 3 #ext:ekw:gradeLevel:id = <bogus value> -> 0 (twice) #l = ekw -> 33 (COUNT agrees: 33) kinds only, no tag filter -> 100 (page limit) ``` The bogus-value control is part of the evidence, not decoration: if a bogus value returned the same count as a real one, the filter would be a no-op and the number meaningless. ``` phpunit (shared/, --exclude-group integration) OK (15 tests, 22 assertions) ``` Verified **non-vacuous**: delegating `setTag` back to `parent::setTag` fails 3 of the 15. One test asserts that the *upstream* `Filter` still rejects `#ext:...`. If that test ever fails, upstream has widened its regex and `ExtendedFilter` can be deleted. ## Follow-ups, not in this PR - Upstream PR to widen the regex in `nostrver-se/nostr-php`, so this class can eventually go away. - `swentel/nostr-php` is pinned `>=1.5 <1.9.3` and 1.9.4 is out. Upgrading pulls in the replacement for the abandoned `leigh/chacha20` and a request timeout property, but raises the floor to **PHP 8.2** — `CLAUDE.md` documents the stack as PHP 8.1+, so that is a redistributability decision, not a mechanical bump.

laoc added 1 commit

2026-07-30 19:54:56 +00:00

fix(shared): accept namespaced ext: tag keys in relay filters 0732d9db62

swentel/nostr-php validates every filter tag name against `/^#[a-z_-]+$/i`
(Filter::validateTagName), so the colons in the NIP-AMB extension grammar are
rejected: `#ext:ekw:schoolType:id` throws a RuntimeException before the REQ is
ever built. NostrFetcher::fetch() passes `$filterParams['tags']` straight to
setTag(), so every ext: facet filter — the primary filter mechanism for the
Materialpool plugin — was unreachable.

The regex is byte-identical in 1.9.2 (vendored), 1.9.3, 1.9.4 and upstream
main, so a version bump does not fix it.

validateTagName is private and cannot be overridden; setTag is public and is
its only caller inside its own body, so ExtendedFilter overrides setTag and
setTags and inherits everything else — dynamic property storage, toArray(),
and therefore the generated REQ. The wire format stays the parent's, which is
what makes the override safe.

Single-letter tags are unaffected: `#l`, `#L` and `#d` already passed the
upstream regex and still do.

Verified against wss://amb-relay.edufeed.org through NostrFetcher itself:

    #ext:ekw:gradeLevel:id = <real value>   ->   3
    #ext:ekw:gradeLevel:id = <bogus value>  ->   0  (twice)
    #l = ekw                                ->  33  (COUNT agrees: 33)
    kinds only, no tag filter               -> 100  (page limit)

A bogus value returning the same count as a real one would mean the filter is
a no-op, so the control is part of the evidence, not decoration.

phpunit: 15/15 in shared/. Verified non-vacuous — delegating setTag back to
parent::setTag fails 3 of them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

laoc merged commit 1c48089bfa into main

2026-07-30 20:55:25 +00:00

laoc referenced this pull request from a commit

2026-07-30 20:55:27 +00:00

fix(shared): accept namespaced ext: tag keys in relay filters (#1)

laoc referenced this pull request

2026-07-30 20:58:39 +00:00

chore(deps): upgrade swentel/nostr-php 1.9.2 -> 1.9.4, raise floor to PHP 8.2 #2