Conversation
1556a61 to
eb503d6
Compare
… rules Add header-based matching to the access-policy engine: a Block/Restrict rule can target a named request header with equals/contains/notEquals/ notContains (the negated operators back an allow-list — block unless the header matches). Substring contains and per-rule operators can't be a Redis TAG query, and an allow-list rule must fire even when the request omits the header, so the condition (headerName/headerValue/headerOperator) is evaluated in code against the raw request headers; an indexed headerMatch sentinel makes every header rule a matching candidate. Checked at both the request-time block middleware and the verify-path hard block, and worth one point of specificity (mirroring the other scalar dimensions). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
eb503d6 to
8b71e74
Compare
Conflicts resolved against the access-rule verdict caching that landed on main (HardBlockVerdictCache + per-request memo): - getPrioritisedAccessRule now takes main's GetPrioritisedAccessRuleOptions (blockOnly / requestMemoHost / skipCache) plus this branch's trailing requestHeaders argument. - The cache/memo now stores the *candidate* list from findRules rather than the ranked result. hardBlockCacheKey is derived from the user scope alone, which carries no notion of an arbitrary request header, so caching a ranked list would serve one request's header-rule verdict to another request with the same scope but different headers. Ranking (which is where accessRuleHeaderMatches runs) is now done per call, outside the cache. - blockMiddleware passes both requestMemoHost and the normalised headers.
undici is declared in dev/prosoponator-bot but never imported anywhere in the package, so `lint:refs` reports it as an unnecessary dependency. It only surfaces downstream: captcha-private pins a captcha commit that predates the dependency being added, so its lint:refs never saw it, and any pin bump past that point fails the check. Removing the declaration rather than ignoring it in the downstream lint command, since the dependency is genuinely unused.
…on-access-rules # Conflicts: # dev/prosoponator-bot/package.json # package-lock.json
|
No updates since Nothing is lost — Converting to draft disables auto-merge, so PRs with auto-merge enabled are excluded from this entirely. |
|
No updates since Nothing is lost — Converting to draft disables auto-merge, so PRs with auto-merge enabled are excluded from this entirely. |
|
No updates since Nothing is lost — Converting to draft disables auto-merge, so PRs with auto-merge enabled are excluded from this entirely. |
|
No updates since Nothing is lost — Converting to draft disables auto-merge, so PRs with auto-merge enabled are excluded from this entirely. |
Paired with the portal PR in captcha-private (#3738). Merge that one after this.
What
Adds an arbitrary-header match dimension to the user-access-policy engine, so a
Block/Restrictrule can target a named request header. Companion to theosdimension (#2786) but for headers, with substringcontainsand allow-list support.Design
Header matching can't be a Redis TAG query — substring
containsand per-rule operators aren't expressible, and an allow-list rule must fire even when the request omits the header. So:headerName,headerValue,headerOperator(equals/contains/notEquals/notContains), plus aheaderMatchsentinel.headerMatch(always"1"on every request scope — seegetRequestUserScope, and on every header rule) is the one indexed field. It makes every header rule a matching candidate for every request; Redis narrows nothing else about it.accessRuleHeaderMatches) against the raw request headers, threaded intoruleAppliesviagetPrioritisedAccessRule/getPrioritisedAccessPolicies. Checked at both the request-time block middleware and the verify-path hard-block.notEquals/notContains= "block unless the header matches", including when it's absent) — the same trick the OS allow-list uses to block the complement. The portal owns that desugaring.headerMatchonly), mirroring the other scalar dimensions.Interaction with the verdict cache (from the main merge)
HardBlockVerdictCachelanded on main after this branch was opened. Its key,hardBlockCacheKey(clientId, userScope, blockOnly), is built from the user scope alone — it carries no notion of an arbitrary request header.Caching the ranked list under that key would let one request's header-rule verdict be served to a different request that shares a user scope but sends different headers, for the length of the cache TTL. So the merge moves the cache boundary: the cache and the per-request memo now store the candidate list from
findRules(the Redis round-trip is what the cache exists to absorb), andrankCandidateRules— which is whereaccessRuleHeaderMatchesruns — is called per request, outside the cache.Net effect: same number of storage calls, ranking is no longer cached. Cache
size()semantics are unchanged (still one entry per key), so the storm integration tests are unaffected.Unrelated commit riding along: the unused
undicidependency748cf900fdropsundicifromdev/prosoponator-bot. It is declared there but never imported anywhere in the package, solint:refsreports it as an unnecessary dependency.It is not visible on captcha CI, only downstream: captcha-private pins a captcha commit predating the dependency being added, so its
lint:refsnever saw it, and any pin bump past that point fails the check — which is what blocked #3738. Removed rather than added to the downstream lint's ignore list, since it is genuinely unused. Deliberately placed before the main merge in this branch's history so #3738 can pin it without also taking the v3.7.6 package-version bumps (see that PR for why).Notes / trade-offs
redisRulesSplitQuery.tsenumerates its scope fields in a hardcoded list that predates bothosandheaderMatch. Header rules are still fetched on the hot path, but via theno-user-scopefall-through probe rather than a dedicated one, so they share that probe'sSPLIT_MAX_CANDIDATES_PER_SUB(500) budget withosrules and genuine client-wide rules. Worth deriving that list fromuserScopeSchemathe wayredisRulesQuery.tsdoes — happy to do it here or as a follow-up.os/countryCode, header rules are low-specificity, so in an account with many higher-specificity rules they can be crowded out of a candidate cap. For a deny-list that loses a block; for an allow-list it means the gate silently stops enforcing, which is the more consequential direction.requestHeaderscurrently defaults to{}onrankCandidateRules/getPrioritisedAccessRule/getPrioritisedAccessPolicies. A caller that omits it gets "no header rule matches" with no error. Both real call paths pass it; consider making it required so a future one can't skip it.headerName/headerValue/headerOperatorare indexed only to satisfy the schema exhaustiveness check; their index entries are never queried.ismissing(@headerMatch).osshipped the same way in feat(user-access-policy): OS match dimension for access rules #2786, so the rebuild path handles it — just land this before anything writes header rules.Tests
headerMatch.unit.test.ts— operator semantics (incl. absent-header and negation cases).blacklistRequestInspector.unit.test.ts—rankCandidateRuleswith deny/allow header rules + sentinel gating.redisRulesQuery/transformRulesnapshots for the new fields.CI note:
tests/lint/typecheck/cypressare not reportingThose four workflow runs come back
conclusion: action_requiredwith zero jobs, so the suites never execute. This is not specific to this PR — it started repo-wide this morning and also hitci/mobile-demo-ci:fix/error-label-selectableci/stale-pr-to-draftci/mobile-demo-ciPOST /actions/runs/{id}/approveis rejected ("not from a fork pull request or queued by the Actions bot"), a manualworkflow_dispatchcomes backaction_requiredjust as fast,actions/permissionsshowsenabled: true/allowed_actions: all, and there are no pending deployments. So it needs someone with org/repo admin to look at whatever changed between 08:40 and 09:19 — the reportingcheckcontexts that do pass come fromchangesets.yml, not from the real suites.Because of that, this branch's suites were run locally against the merged tree instead:
@prosopo/user-access-policy— 103/103 unit, 46/46 integration (incl. the retry-storm benchmark).@prosopo/provider— 1024/1024 unit across 72 files, 18/18 integration (incl.blacklistRequestInspector.storm).npx turbo run typecheckacross the captcha workspace — clean (24/24 tasks).biome check .— clean.lint:refs— exit 0.