Skip to content

test(authz): pin the GET-exemption set by membership, not by count (#11531) - #11580

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
ntdatt812:fix/11531-get-exemption-cardinality-pin
Aug 26, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
ntdatt812:fix/11531-get-exemption-cardinality-pin

Conversation

@ntdatt812

Copy link
Copy Markdown
Contributor

The red test

Unit Tests fast-path fails on every branch:

✖ LOCAL_ONLY_API_GET_EXEMPTIONS has exactly 1 entry
  AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
    actual: 2, expected: 1

#11531 (e48ffd38d, yesterday)
added /api/tunnels/cloudflared to LOCAL_ONLY_API_GET_EXEMPTIONS and shipped
tests/unit/authz/route-guard-tunnel-processes-local-only.test.ts alongside it. What it
did not touch was the #5083 sibling four directories over, which pins the set's
cardinality at 1.

This is the test that is wrong, not the code

Worth stating plainly, because "align the assertion to current behaviour" is usually how
a real defect gets buried. Here the production change is the reviewed one:

  • GET /api/tunnels/cloudflared is tunnel status — no spawn.
  • POST on the same path still spawns cloudflared and stays local-only.

route-guard-tunnel-processes-local-only.test.ts asserts both, and it passes today. The
exemption was deliberate; only the count in the older file rotted.

Why membership instead of a bigger number

Bumping 1 to 2 would go green, but it would keep a guard that cannot do its job. A
size assertion:

  • cannot say which path appeared. actual: 2, expected: 1 sends the next author
    hunting for a diff the test already had in hand.
  • cannot see a substitution at all. Swap /api/system/version for a spawn-capable
    route and the size stays 1 — a security-relevant edit passes silently.

Pinning the exact membership keeps the "must not grow by accident" property, fails just
as loudly on an addition, additionally catches the swap, and prints the offending path.
Both entries are now listed with the reason each is safe for a read-only method, so the
next person to add one is told what the bar is.

Verification

Three states, same file:

# base branch, unmodified
✖ LOCAL_ONLY_API_GET_EXEMPTIONS has exactly 1 entry     actual: 2, expected: 1
ℹ pass 18  ℹ fail 1     (with the tunnel sibling: 19 tests)

# with this change
✔ LOCAL_ONLY_API_GET_EXEMPTIONS holds exactly the reviewed paths
ℹ pass 15  ℹ fail 0

# guard still bites — a third entry added to the set by hand
✖ LOCAL_ONLY_API_GET_EXEMPTIONS holds exactly the reviewed paths
  + actual - expected
    [
  +   '/api/db-backups/exportAll',
        '/api/system/version',
        '/api/tunnels/cloudflared'
    ]
  ✖ GET /api/db-backups/exportAll still local-only (spawns tar)     ← also fires
ℹ pass 13  ℹ fail 2

# guard catches a swap, which `size` could not — cloudflared replaced by exportAll
✖ LOCAL_ONLY_API_GET_EXEMPTIONS holds exactly the reviewed paths
  + actual - expected
  +   '/api/db-backups/exportAll',
        '/api/system/version',
  -   '/api/tunnels/cloudflared'

Whole directory, after the change:

node --import tsx/esm --import ./open-sse/utils/setupPolyfill.ts \
     --import ./tests/_setup/isolateDataDir.ts --test --test-force-exit \
     --test-concurrency=4 "tests/unit/authz/*.test.ts"
# tests 250 | pass 248 | fail 2

The two remaining failures are client-api-policy-fallback and management-policy,
both failing in their test.after rmSync of a tmp DATA_DIR with EPERM on this
Windows host — identical on the unmodified base branch, unrelated to this change.

npx eslint <file>      # No issues found
npx prettier --check   # clean

Test-only; no production code touched.

…iegosouzapw#11531)

diegosouzapw#11531 added `/api/tunnels/cloudflared` to `LOCAL_ONLY_API_GET_EXEMPTIONS` and
shipped its own guard for it, but the diegosouzapw#5083 sibling asserts
`LOCAL_ONLY_API_GET_EXEMPTIONS.size === 1` — so `Unit Tests fast-path` has been red
on every branch since:

    ✖ LOCAL_ONLY_API_GET_EXEMPTIONS has exactly 1 entry
      actual: 2, expected: 1

The production side is correct and reviewed: GET on that path is tunnel status,
and POST still spawns cloudflared and stays local-only
(`route-guard-tunnel-processes-local-only.test.ts` proves both).

The stale assertion is the defect, and a cardinality pin is the wrong shape for
what it was guarding. It cannot say WHICH path appeared, and it cannot see a
substitution at all — swapping `/api/system/version` for a spawn-capable route
keeps the size at 1 and passes. Asserting the exact membership keeps the "must not
grow by accident" guard, fails just as loudly on an addition, additionally catches
a swap, and names the offending path in the diff. The two entries are listed with
the reason each is safe for a read-only method.
@diegosouzapw
diegosouzapw merged commit 8cb2d0c into diegosouzapw:release/v3.8.51 Aug 26, 2026
10 of 16 checks passed
diegosouzapw pushed a commit that referenced this pull request Aug 26, 2026
Merged via /merge-batch (lote 2026-08-26 batch 2, v3.8.51). 7 conflitos, todos triviais/duplicados (mesmos base-reds já corrigidos por PRs paralelas mergeadas neste lote — #11580/#11582/#11583/#11585/#11588/#11589/#11590/#11591/#11609): mantida a versão já validada nesses casos. Validado: 68/68 testes passando. Obrigado por resolver os base-reds.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…iegosouzapw#11531) (diegosouzapw#11580)

Merged via /merge-batch (lote 2026-08-26, v3.8.51). Boarded no worktree combinado junto com outras ~30 PRs; validação única: typecheck/complexity/cognitive-complexity/changelog-integrity verdes, file-size rebaseado onde necessário (crescimento legítimo), lint com os mesmos 228 achados pré-existentes confirmados via sonda contra o tip puro (não introduzidos por este lote), e ~370 testes focados (unit + vitest) passando. Obrigado pela contribuição.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…osouzapw#11608)

Merged via /merge-batch (lote 2026-08-26 batch 2, v3.8.51). 7 conflitos, todos triviais/duplicados (mesmos base-reds já corrigidos por PRs paralelas mergeadas neste lote — diegosouzapw#11580/diegosouzapw#11582/diegosouzapw#11583/diegosouzapw#11585/diegosouzapw#11588/diegosouzapw#11589/diegosouzapw#11590/diegosouzapw#11591/diegosouzapw#11609): mantida a versão já validada nesses casos. Validado: 68/68 testes passando. Obrigado por resolver os base-reds.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants