decentraland / builder-server / 32492101680

72%

master: 72%

Build Type

Pull #809

github

Committed by LautaroPetaccio

Commit Message

fix: refuse a rejected signer before the legacy fallback can clear it

The signer gate ran inside the `verify()` attempt, so its `throw` landed in
the very `catch` that retries the request against the legacy `method:path`
signature. That fallback binds no metadata, so on success it returned an
address and the refusal disappeared.

The scene signer itself was never exposed: the raw-header check ahead of the
`try` already refused it. What leaked was the rest of what the gate covers --
a signer that is not canonical. `signer: 'Dcl:Explorer'` delivered with a
valid legacy signature resolved to an address; it now rejects.

Fixed by moving the gate rather than patching around it. The pre-`try` check
now applies `isNotSceneSigner` to the parsed metadata header, and the check
inside the `try` -- whose throw was the bug -- is gone. One predicate, one
place, and it runs before the fallback is reachable. This is strictly
stricter than the `trim().toLowerCase()` comparison it replaces: `rejectIfSigner`
refuses the scene signer, any non-canonical spelling, and a duplicated
`signer` key, and folds nothing.

The metadata header is the right thing to gate on because it is what
handlers receive; `verify()` returns those same bytes, so nothing is lost by
checking earlier.

Tests: the mocked specs set `authMetadata` on `verify()`'s return with empty
headers, which no longer describes where the value comes from, so they now
deliver the header. Adds regression coverage for a valid legacy signature
carrying metadata the gate refuses -- the case that used to resolve -- plus a
canonical allowed signer, so the gate is not silently refusing legacy callers.

Also sets `.nvmrc` to 24.18.0, matching the Dockerfile, and declares
`engines.node >=22.0.0`, the floor the middleware's dependencies require. It
read 20, which no longer satisfies the dependency graph.

Not covered: a re-cased `signer` *key* is still accepted, because 6.2.0 does
not refuse folded keys. That is core-libs#74... (continued)

Pull Request Pull Request #809: fix(auth): upgrade to crypto-middleware 6 and refuse non-canonical signers

Coverage Stats

1442 of 2216 branches covered (65.07%)

Branch coverage included in aggregate %.

6 of 7 new or added lines in 1 file covered. (85.71%)

1 existing line in 1 file now uncovered.

3327 of 4393 relevant lines covered (75.73%)

48.64 hits per line