Skip to content

WAF component: rule-table-driven request filtering with live updates, reserved v1 schema, shadow/monitor mode, and activation gating#517

Open
kriszyp wants to merge 13 commits into
kris/waf-re2-depfrom
kris/waf-prototype
Open

WAF component: rule-table-driven request filtering with live updates, reserved v1 schema, shadow/monitor mode, and activation gating#517
kriszyp wants to merge 13 commits into
kris/waf-re2-depfrom
kris/waf-prototype

Conversation

@kriszyp

@kriszyp kriszyp commented Jul 2, 2026

Copy link
Copy Markdown
Member

Summary

Adds a Web Application Firewall to Harper as an HTTP middleware component (harper-pro): rule-based request filtering (IP/CIDR, method, path, header, query) evaluated against a compiled, immutable matcher, with rules living in a replicated system.hdb_waf_rules table so changes propagate cluster-wide in seconds with no restart. Non-matching hot-path cost is ~1.2 µs at 1,000 rules. This is the enforcement plane of the two-plane design in the WAF design proposal.

This PR is the prototype plus the v1 "wave 2" that turns it into a shippable v1 (schema-freeze content + operational controls). Rule management is via dedicated super_user-only registered operations (add_waf_rule/alter_waf_rule/drop_waf_rule/list_waf_rules/set_waf_mode), not generic CRUD.

What's in it

Prototype (baseline): anchored compiled matcher (each rule indexed on its most-selective condition, residual predicate on hit; CIDR interval arrays, path trie, combined-regex pre-gate), subscription-driven live recompile with atomic matcher swap, fail-open posture with rate-limited match logging, block/log/score actions.

v1 additions in this PR:

  • Canonicalize-before-match — request path and rule path.exact/path.prefix literals are normalized the same way (bounded percent-decode + RFC 3986 dot-segment resolution + duplicate-slash collapse) before matching, closing /./, //, /../, %2e/%2F, and double-encoding path-rule bypasses. No case-folding (case-sensitive paths are legitimate). Fast-path early-return keeps already-canonical paths allocation-free.
  • Reserved schema slots (the near-irreversible freeze content) — validated shape now, even where v1 doesn't enforce: match ja4/ja4h/model/agent; actions challenge/serve/drop; rule shadow/activation/scope/provenance/rateLimit.
  • Deferral — a valid rule using a reserved-but-unenforced feature compiles OUT into a separate unsupportedRules map (distinct from invalidRules) with a reason, so it never silently under-enforces and never masquerades as malformed.
  • Shadow + global monitor mode — per-rule shadow (matches, previews a would-block, never enforces) and a fleet-wide mode via a replicated __waf_control__ sentinel row (enforce/monitor/off); off is a pass-through kill switch, monitor downgrades all block/score enforcement to would-block previews while log rules still log.
  • Activation gating — compile-time activation selector (nodes/regions/tags vs the local node identity); unarmed rules are compiled out. Doubles as staged/canary rollout.
  • Per-rule telemetry — hit counters + last-matched, persisted across recompiles (per-worker).
  • Explicit user write-context in the rule ops so audit records attribute the change (belt-and-suspenders with harper#1592, which fixes attribution generically).

Where to look

  • Reserved schema shapes (waf/rules.ts) — this is what freezes; the shapes were reviewed/approved against the design doc's freeze checklist. Reopening later is a migration.
  • canonicalizePath / removeDotSegments (waf/matcher.ts) — correctness-critical; RFC-conformant trailing-slash handling and root-escape safety, with bypass-attempt tests.
  • evaluate() real/shadow track split (waf/matcher.ts) — no shadow enforcement leak into the real decision; monitor suppresses block/score enforcement but not log output; the 99% no-candidate path stays allocation-free.
  • __waf_control__ containment — pulled out before compile, excluded from list_waf_rules, add/alter reject the reserved id, mutable only via super_user set_waf_mode with a mode allowlist.

Intentionally NOT enforced in v1 (reserved only)

challenge/serve/drop actions, ja4/ja4h/model/agent match fields, and rateLimit are schema-reserved and deferred (a rule using them is compiled out with a reason). scope/provenance are validated-and-persisted metadata, not enforced. RE2 for operator regex is a follow-up (decided: node-re2; the prototype uses JS RegExp behind a validator). Body-phase (requestBody) rules are not evaluated. Compliance definition-of-done (audit logging, redaction, health signal, starter pack) is tracked in #518.

Tests & review

107 WAF unit tests passing; build + oxlint clean on all waf/ files. Cross-model reviewed (thorough): Codex + Gemini outside-model passes + a Harper-domain trace — no blockers. Three significant concerns found and fixed in this PR (RFC trailing-slash divergence, missing canonicalize fast-path, monitor mode suppressing log output). One Gemini "blocker" (null-deref) was a diff-only false positive (guard intact). Coverage note: the first Codex run produced empty output and was re-run clean, so the outside-model coverage is genuine two-model, not one.

Depends on harper#1592 (registered-op audit attribution) for the audit who to also resolve via ambient context.

Generated by an LLM (Claude — Fable 5), reviewed by Kris.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new Web Application Firewall (WAF) component, featuring an optimized rule matcher, dedicated operations-API endpoints for rule management, and early HTTP middleware integration. The code review identified several critical security and robustness issues, including IP parsing discrepancies in IPv4 and IPv6, a vulnerability in CIDR bits parsing, a potential memory leak in the middleware's singleton request adapter, and missing validations for rule types and empty match arrays.

Comment thread waf/matcher.ts
Comment thread waf/matcher.ts Outdated
Comment thread waf/matcher.ts Outdated
Comment thread waf/matcher.ts
Comment thread waf/waf.ts Outdated
Comment thread waf/rules.ts
Comment thread waf/rules.ts
Comment thread waf/rules.ts Outdated
Comment thread waf/ruleOperations.ts Outdated
Comment thread waf/waf.ts Outdated
@claude

claude Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

One blocker remains (re-raised): rule.score validation at waf/rules.ts:239 still uses typeof rule.score !== 'number', accepting NaN/Infinity. 269ed4f fixed rateLimit.limit, rateLimit.windowMs, and model.threshold with Number.isFinite but missed score. A score rule with score: NaN compiles, then permanently poisons totalScore to NaNNaN >= scoreThreshold is always false, so the WAF's score accumulation can never block. Fix and test case on the inline thread.

The two new commits (269ed4f and 87a0250) otherwise look clean: method upper-casing, activation empty-array guard, drop_waf_rule reserved-id guard, blockStatus 4xx/5xx constraint, telemetry pruning, lazy getThisNodeName(), and the RE2 refactor (closing the ReDoS vector, deleting the now-unnecessary alternation gate) all look correct.

kriszyp added a commit that referenced this pull request Jul 4, 2026
Thread request.hdb_user as an explicit { user } context arg into table.put /
table.delete from add/alter/drop_waf_rule so rule-change audit records are
attributed even if this ships before core's ambient-context fix (harper#1592).
Explicit context takes precedence over ambient, so it is harmless once #1592
lands and decouples #517 from that merge order. Widen WafRuleStore's put/delete
signatures and assert the context in the operations unit test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kriszyp kriszyp changed the title WAF component: rule-table-driven request filtering with live rule updates (prototype) WAF component: rule-table-driven request filtering with live updates, reserved v1 schema, shadow/monitor mode, and activation gating Jul 4, 2026
@kriszyp kriszyp marked this pull request as ready for review July 4, 2026 12:30
@kriszyp kriszyp requested a review from a team as a code owner July 4, 2026 12:30
@kriszyp kriszyp requested review from Ethan-Arrowood, cb1kenobi and hdbjeff and removed request for a team July 4, 2026 12:30
kriszyp added a commit that referenced this pull request Jul 4, 2026
- alter_waf_rule: copy existing fields via the PATCHABLE_FIELDS allowlist
  instead of object-spread; TrackedObject attributes are inherited getters
  (own-enumerable keys are empty) so spread dropped every un-patched field
- parseIpv4: reject octets with leading zeros (010.0.0.1 parse ambiguity)
- parseIpv6: require strictly hex groups (parseInt tolerated '123g')
- validateRule: reject empty match.ip/headers/query arrays (dead rules)
- parseQueryString: decode name and value in independent try/catch blocks
- waf.ts: attach a .catch to the detached subscription IIFE

Adds unit tests for each, including a TrackedObject-shaped fake table that
exercises the inherited-getter read path for alter_waf_rule.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread waf/waf.ts Outdated
Comment thread waf/rules.ts Outdated
Comment thread waf/ruleOperations.ts
Comment thread waf/rules.ts Outdated
Comment thread waf/rules.ts Outdated
Comment thread waf/waf.ts
function recompile() {
const rules = readAllRules();
// mode precedence: replicated control row > config fallback > 'enforce'.
const mode = controlMode ?? options.mode ?? 'enforce';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low: scoreThreshold is per-node config, not replicated — identical score rules can block on one node and pass on another

The global mode was deliberately made a replicated control row so enforcement is cluster-consistent, but scoreThreshold (waf.ts:215) stays a per-node config value fed into compileRules. Two nodes with divergent or lagging scoreThreshold compile the same score-action rules into different block behavior — the accumulated-score block fires on one node and passes on another for the same request — undermining consistent cluster-wide enforcement.

Suggested fix: carry scoreThreshold on the replicated control row alongside mode, or document that it must be identical cluster-wide and warn at startup if it is unset while score rules exist.


Generated by Barber AI

Comment thread waf/matcher.ts
Comment thread waf/waf.ts Outdated
Comment thread unitTests/waf/component.test.mjs
Comment thread waf/matcher.ts
* activation on this node are silently omitted (not counted as invalid or unsupported).
* When options.mode is 'off' a pass-through (isEmpty) matcher is returned immediately (kill switch).
*/
export function compileRules(rules: WafRule[], options: CompileOptions = {}): WafMatcher {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleanup: matcher is heavily specialized for a super_user-authored v1 rule set

For rules authored only by super_user (realistically tens to low-hundreds), this file carries a character-level prefix trie, a sorted-typed-array interval index with prefix-max-end binary search, a combined-alternation regex pre-gate with backreference exclusion, and an "invert header iteration when >16 anchors" heuristic — a large, high-cognitive-load body optimizing a rule count unlikely to be the bottleneck, while the TODOs already concede the regex path must be redone with RE2 later. The benchmarks justify the design at 1000 rules, so this is a judgment call, not a defect — flagging it against the simplicity bias.

Suggested fix (if revisited): keep the isEmpty/currentMatcher===null hot-path short-circuit and the anchor+residual split, but replace the specialized structures with plain arrays and linear scans (still allocation-free on the no-match path); reintroduce a specific index only when profiling shows one condition type dominates. Removes several hundred lines.


Generated by Barber AI

kriszyp added a commit that referenced this pull request Jul 9, 2026
…uard, map pruning, fail-open ordering (#517)

Addresses cb1kenobi's review on #517:
- upper-case the incoming request method before matching (rule methods are
  upper-cased at compile, so a lowercase/mixed verb bypassed method anchors)
- reject an empty activation.nodes/regions/tags selector array (empty gating
  array is a silent cluster-wide no-op); scope (non-gating) still allows empty
- drop_waf_rule now rejects the reserved __waf_control__ id (was deletable)
- reserved numeric fields (rateLimit.limit/windowMs, model.threshold) require
  finite/positive values instead of accepting NaN/Infinity/negatives
- constrain blockStatus to 4xx/5xx so a "block" can't return 2xx/3xx; block
  reason phrase falls back by status class (5xx -> Internal Server Error)
- prune module-level ruleStats + LogRateLimiter windows on recompile so they
  don't grow unbounded and a reused id can't inherit stale telemetry
- resolve getThisNodeName() lazily inside recompile() so a failure fails open
  under the O2 retry guard instead of throwing out of start()
- tests: control-row -> global-mode seam (sentinel stripped, monitor downgrade,
  off kill switch), block short-circuit contract, method-casing, drop guard,
  stats pruning, and the new validation guards

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread waf/rules.ts
if (rule.priority != null && typeof rule.priority !== 'number') errors.push('priority must be a number');
if (!VALID_PHASES.has(rule.phase)) errors.push(`unknown phase ${JSON.stringify(rule.phase)}`);
if (!VALID_ACTIONS.has(rule.action)) errors.push(`unknown action ${JSON.stringify(rule.action)}`);
if (rule.action === 'score' && typeof rule.score !== 'number') errors.push('action "score" requires a numeric score');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1. score validation allows NaN / Infinity, creating a silently-dead rule

What: typeof rule.score !== 'number' accepts NaN and Infinity because typeof NaN === 'number'. Commit 1c937ab explicitly tightened every other numeric field in this PR to Number.isFinite (rateLimit.limit, rateLimit.windowMs, model.threshold), but score was not included.

Why it matters: A score-action rule with score: NaN passes validateRule() with no errors, compiles normally, and appears in list_waf_rules. But in evaluate(): totalScore += NaNtotalScore = NaN, and NaN >= scoreThreshold is always false — the rule silently never contributes to a block. An operator who configures a score rule gets no validation error and no log warning; they just observe the WAF not blocking as expected.

Suggested fix:

Suggested change
if (rule.action === 'score' && typeof rule.score !== 'number') errors.push('action "score" requires a numeric score');
if (rule.action === 'score' && !(Number.isFinite(rule.score) && (rule.score as number) > 0)) errors.push('action "score" requires a positive finite numeric score');

kriszyp and others added 12 commits July 11, 2026 07:48
…ates, request middleware

Prototype of a WAF built-in component to validate the design and measure hot-path cost:
- waf/rules.ts: rule document shape (ip CIDR / method / path / header / query conditions;
  block / log / score actions) + validation, ReDoS TODO (RE2 for production)
- waf/matcher.ts: immutable compiled matcher — rules anchored on their most selective
  condition (IPv4 sorted intervals, path exact map, path-prefix char trie, combined-regex
  pre-gate, header-name map with request-side iteration inversion, method map) with residual
  predicates; non-matching requests allocate nothing
- waf/waf.ts: builtin component (start/startOnMainThread) — defines data.waf_rule on both
  thread kinds, subscribes to the table, debounces recompiles (100ms), swaps the matcher
  reference atomically; registers non-async middleware with before: 'authentication' + runFirst
- micro-bench (benchmarks/waf/matcher-bench.mjs): ~50ns/op @10 rules, ~1.2µs/op @1000 rules
  non-matching; e2e smoke showed no measurable latency delta (~66µs p50 either way)
- unit tests: 22 cases over CIDR/path/header/query/method matching, actions, priority,
  validation, recompile swap

Not for merge — prototype exploration on a worktree branch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t operations

Generic CRUD operations cannot touch system tables (core allowlists only
hdb_nodes/hdb_role/hdb_user for super_user), so rule management follows the
add_node / install_usage_license pattern: dedicated registered operations
(add/alter/drop/list_waf_rule(s)) on the main thread, with a defense-in-depth
super_user check in the handlers plus rule validation and a patch-field
allowlist (the ops server attaches metadata to the request body).

Verified over the wire: super_user CRUD + live block/disable round-trip works
from the system table; a non-super_user with working data-CRUD permissions is
denied on the dedicated ops (400 op-not-found from core auth) and on generic
insert/read of the system table (403); system tables replicate by default so
rules distribute cluster-wide.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ion tests

Matcher (waf/matcher.ts, waf/rules.ts):
- M1 stage-then-commit: a rule's index insertions are staged in locals and merged into the
  shared structures only after the validity gate passes, so a rejected rule's ruleIndex is
  never aliased by stale entries when the next valid rule reuses it (was: bad-ip+valid-path
  rule corrupted a following rule).
- M2 parseCidr rejects non-canonical bit strings ('10.0.0.0/' → was /0 block-all; '/0x10',
  '/1e1', whitespace) via a base-10-integer regex; validateRule now calls parseCidr so its
  verdict matches the compiler (also M8).
- M3 backreference regexes (\1..\9) are excluded from the combined pre-gate and always scanned,
  so alternation group-renumbering can't produce a false-negative that disarms a sibling rule.
- M4 evaluate() never throws on malformed requestInfo: null/non-string ip and missing/non-string
  path are treated as absent; normalizeIp and header residuals are null-safe.
- M7 header residual lowercases the name to match the anchor; M9 rejects empty match strings;
  M8 type-checks enabled/priority. M5/M6 documented as deferred TODOs in code.

Component (waf/waf.ts):
- O1 middleware wraps evaluate() and FAILS OPEN on internal error; resets shared requestInfo in
  finally so a throw can't retain request headers.
- O2 initial compile can't throw out of start(): on failure the middleware still registers
  (pass-through) and a backoff retry runs.
- O3 per-rule log rate limit (default 100/60s) with a suppressed-count summary.
- O4 opaque block body (no ruleIds to client), correct reason phrase, JSON content type.
- O5/O7 subscribe-before-scan so a rule committed in the scan window isn't missed.
- O8 stop()/reset teardown clears timers and breaks the subscription loop on reload.

Tests: adversarial.test.mjs (M1-M9 exploits incl. never-throws fuzz) + component.test.mjs
(O1-O4, enabled:false, teardown). 47 WAF unit tests pass; live re-verify confirms opaque 403
+ JSON content-type + live alter/drop against system.hdb_waf_rules.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hort-circuits, telemetry doesn't)

Per compliance review: a request rejected by a block rule must still emit
log records for any log-action rules that matched it (forensic/audit value).
The matcher surfaces matchedLogRuleIds on block and score-threshold-block
decisions; the middleware records them (rate-limited) alongside the block,
which itself still short-circuits enforcement. Opaque client response
unchanged. Adds regression coverage.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ypasses

Add canonicalizePath (bounded percent-decode, RFC 3986 remove_dot_segments,
duplicate-slash collapse; case preserved) and apply it to the request path in
the middleware and to authored path.exact/path.prefix literals at compile time,
so rules and requests share one normalized matching space. Closes /admin%2F,
/./admin, //admin, /admin/../admin, and double-encoding bypasses of path rules.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Track a per-rule hitCount and lastMatched in a module-level map keyed by rule
id, so counters survive the matcher reference swap on each recompile. Updated
only in evaluate()'s slow path for the final matched set (any action), keeping
the 99% no-candidate path allocation-free. Exposed via getRuleStats() /
resetRuleStats() and a matcher.getStats() convenience. Per-worker,
process-lifetime counters; cross-worker aggregation rides on #518.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Thread request.hdb_user as an explicit { user } context arg into table.put /
table.delete from add/alter/drop_waf_rule so rule-change audit records are
attributed even if this ships before core's ambient-context fix (harper#1592).
Explicit context takes precedence over ambient, so it is harmless once #1592
lands and decouples #517 from that merge order. Widen WafRuleStore's put/delete
signatures and assert the context in the operations unit test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n gating

Wave 2 rule-engine additions (all backward-compatible, additions optional):

Reserved schema slots (rules.ts) with full SHAPE validation so a slot never has
to migrate later: match.ja4/ja4h/model/agent; widened action union
(block/log/score/challenge/serve/drop); top-level shadow, activation, scope,
provenance, rateLimit. New VALID_* sets mirror the existing style.

Deferral (decision a): a rule can be VALID (shape ok) yet use a feature the v1
engine cannot honor. compileRules compiles it OUT and reports it via a new
unsupportedRules map + onUnsupportedRule callback, kept DISTINCT from
invalidRules (malformed) so a deferred count never masquerades as a
malformed-rule count. Deferred = action in {challenge,serve,drop}, a
ja4/ja4h/model/agent match, or rateLimit. scope/provenance are reserved METADATA
in v1 (validated + persisted, NOT enforced, no deferral).

shadow (implemented): a matched shadow rule never enforces; evaluate() splits the
matched set into a REAL track (today's exact logic) and a SHADOW track that
mirrors block/score resolution into a would-block preview (WafDecision.
shadowRuleIds). A real block attaches the preview; a shadow-only would-block
returns a pass-through (action 'log', status 0) carrying it. The 99% no-candidate
path stays allocation-free.

global mode (decision b): CompileOptions.mode — 'off' returns a pass-through
(isEmpty) kill-switch matcher; 'monitor' treats every rule as shadow;
'enforce' is normal (default).

activation gating (decision c): CompileOptions.nodeIdentity arms a rule on this
node only when node/region/tag selectors all match; not-for-this-node rules are
compiled out silently (neither invalid nor unsupported).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…waf_mode op

Component + operations wiring for the wave 2 engine features:

waf.ts recompile() sources node identity (name via getThisNodeName — the same
config accessor replication uses; region/tags from waf config) and passes it plus
the derived mode into compileRules. Mode precedence: replicated control row >
config fallback > 'enforce'. readAllRules pulls the sentinel control row
(id '__waf_control__', { mode }) OUT of the rule list so it is never compiled or
validated as a rule. onUnsupportedRule logs "rule X deferred: ..." and the
compiled-summary line now includes the deferred count and active mode.

Middleware gains a would-block branch: a shadow/monitor match is logged
(rate-limited via the existing LogRateLimiter, keyed on the first shadow rule id)
as "WAF would block ..." and never returns a block response. Riding shadow
previews are also logged on real blocks.

set_waf_mode op (super_user, validates enforce/monitor/off) upserts the control
row; registered in startOnMainThread. add_waf_rule / alter_waf_rule reject the
reserved sentinel id; list_waf_rules filters it out. add_waf_rule defaults
provenance to { origin: 'human', approver: <username> } on a COPY (no input
mutation). PATCHABLE_FIELDS + TABLE_DEFINITION gain the new top-level fields.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…, monitor log)

Three significant findings from the wave-2 cross-model review, plus two cleanups:

1. removeDotSegments dropped the boundary trailing slash (RFC 3986 §5.2.4
   divergence): /admin/. canonicalized to /admin, but a rule literal /admin/ (no
   dot segment to resolve) canonicalized to /admin/, so they never matched. Now a
   final . / .. segment preserves the boundary slash (/admin/.→/admin/,
   /admin/x/..→/admin/, /admin/..→/, /a/b/.→/a/b/); interior dots unchanged.

2. canonicalizePath allocated on every request. Added a zero-allocation fast path
   at the top: a path with no percent-escape, no '//', and no '/.' is already
   canonical and returned as-is (the 99% case) — preserving the matcher's
   allocation-free non-candidate path. A legit '/.'-containing segment
   (e.g. /.well-known) harmlessly takes the slow path and canonicalizes to itself.

3. monitor mode (and per-rule shadow) suppressed log-action output — a log rule
   hit the shadow branch and never reached logIds. log is orthogonal to shadow (it
   never enforces), so it must never be suppressed. evaluate() now collects every
   matched log rule into the real logIds BEFORE the real-vs-shadow enforcement
   split; only block/score go through the would-block track. Net: monitor mode =
   would-block previews for block/score PLUS normal log output (the intended
   visibility-adding dry-run).

Cleanups: folded the duplicate-slash collapse into removeDotSegments' single pass
(removed the separate /\/{2,}/g regex); documented that shadow/monitor matches
still increment hitCount (it is a match counter, not a block counter).

Tests: trailing-slash canonicalization + a regression assertion that /admin/ now
matches /admin/.; monitor-mode + shadow log rules still logged; shadow/monitor
matches still counted as hits.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- alter_waf_rule: copy existing fields via the PATCHABLE_FIELDS allowlist
  instead of object-spread; TrackedObject attributes are inherited getters
  (own-enumerable keys are empty) so spread dropped every un-patched field
- parseIpv4: reject octets with leading zeros (010.0.0.1 parse ambiguity)
- parseIpv6: require strictly hex groups (parseInt tolerated '123g')
- validateRule: reject empty match.ip/headers/query arrays (dead rules)
- parseQueryString: decode name and value in independent try/catch blocks
- waf.ts: attach a .catch to the detached subscription IIFE

Adds unit tests for each, including a TrackedObject-shaped fake table that
exercises the inherited-getter read path for alter_waf_rule.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…uard, map pruning, fail-open ordering (#517)

Addresses cb1kenobi's review on #517:
- upper-case the incoming request method before matching (rule methods are
  upper-cased at compile, so a lowercase/mixed verb bypassed method anchors)
- reject an empty activation.nodes/regions/tags selector array (empty gating
  array is a silent cluster-wide no-op); scope (non-gating) still allows empty
- drop_waf_rule now rejects the reserved __waf_control__ id (was deletable)
- reserved numeric fields (rateLimit.limit/windowMs, model.threshold) require
  finite/positive values instead of accepting NaN/Infinity/negatives
- constrain blockStatus to 4xx/5xx so a "block" can't return 2xx/3xx; block
  reason phrase falls back by status class (5xx -> Internal Server Error)
- prune module-level ruleStats + LogRateLimiter windows on recompile so they
  don't grow unbounded and a reused id can't inherit stale telemetry
- resolve getThisNodeName() lazily inside recompile() so a failure fails open
  under the O2 retry guard instead of throwing out of start()
- tests: control-row -> global-mode seam (sentinel stripped, monitor downgrade,
  off kill switch), block short-circuit contract, method-casing, drop guard,
  stats pruning, and the new validation guards

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ion gate

Point compileRuleRegex at the RE2-backed compileSafeRegex (harper-pro #563)
so operator-supplied patterns match in guaranteed linear time — closing the
ReDoS vector the prototype's `new RegExp(...)` left open (a hostile `(a+)+$`
can no longer stall the event loop).

Because every path-regex is now a non-backtracking RE2, the JS-RegExp
combined-alternation pre-gate and its backreference-safety machinery
(hasBackreference + gateable/ungated split) exist only to make an unsafe
engine fast, so they are deleted: path-regex rules are matched by a plain
linear scan, still allocation-free on the no-match path. For realistic
super_user-authored rule sets this scan is negligible; RE2.Set is the
drop-in native multi-pattern scan if path-regex volume ever dominates.

Contract change: RE2 cannot evaluate backreferences or lookaround, so a rule
using them is now rejected as invalid (same as malformed syntax) rather than
silently compiled. The M3 adversarial tests are rewritten to pin this
rejection and the ReDoS immunity in place of the removed gate's behavior.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kriszyp kriszyp changed the base branch from main to kris/waf-re2-dep July 11, 2026 14:09
@kriszyp kriszyp force-pushed the kris/waf-prototype branch from 1c937ab to 87a0250 Compare July 11, 2026 14:11
Comment thread waf/rules.ts
if (rule.priority != null && typeof rule.priority !== 'number') errors.push('priority must be a number');
if (!VALID_PHASES.has(rule.phase)) errors.push(`unknown phase ${JSON.stringify(rule.phase)}`);
if (!VALID_ACTIONS.has(rule.action)) errors.push(`unknown action ${JSON.stringify(rule.action)}`);
if (rule.action === 'score' && typeof rule.score !== 'number') errors.push('action "score" requires a numeric score');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocker (re-raise from prior run): rule.score accepts NaN/Infinity

typeof NaN === 'number' is true, so score: NaN passes validation, compiles into the matcher, and silently disables all further score accumulation on that request — totalScore += NaN produces NaN, and NaN >= scoreThreshold is always false, so the block threshold is never reached.

269ed4f fixed rateLimit.limit, rateLimit.windowMs, and model.threshold with Number.isFinite, but this line was left unchanged.

Suggested change
if (rule.action === 'score' && typeof rule.score !== 'number') errors.push('action "score" requires a numeric score');
if (rule.action === 'score' && !Number.isFinite(rule.score)) errors.push('action "score" requires a finite numeric score');

A test to pin this:

expect(validateRule({ ...BASE, id: 'x', action: 'score', score: NaN,      match: { ip: '10.0.0.1' } })).to.not.be.empty;
expect(validateRule({ ...BASE, id: 'x', action: 'score', score: Infinity,  match: { ip: '10.0.0.1' } })).to.not.be.empty;

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