Skip to content

Enforcement-core hardening: fail-open & policy-widening fixes (#41)#46

Open
davidcrowe wants to merge 1 commit into
mainfrom
fix/enforcement-core-hardening
Open

Enforcement-core hardening: fail-open & policy-widening fixes (#41)#46
davidcrowe wants to merge 1 commit into
mainfrom
fix/enforcement-core-hardening

Conversation

@davidcrowe

Copy link
Copy Markdown
Collaborator

Fixes #41. Security-review hardening of the OSS enforcement packages. These are reputational/external-consumer correctness (prod's coding-agent hook path uses a private engine), but the repo is the trust artifact.

validatabl-core

  • Schema fail-open (decision.ts): a configured inputSchema was skipped when input was undefined (GET, wrong content-type), so a schema meant to gate a route was a silent no-op. Now the schema always runs; missing input denies.
  • hasScope regex injection (scopes.ts): built a RegExp from the raw scope, so tool.write matched toolXwrite and metachars threw 500s. Now an exact whitespace-delimited token match — no regex.
  • Policy widening (policy.ts): matches was unanchored (search allowed unsafe-search-exec) → bare patterns anchored ^(?:…)$, author anchors honored; in used String() coercion (array field → "a,b" slipped past an allow-list) → strict string membership; empty-conditions rules matched every request → validatePolicySet() rejects them loudly and applyPolicies calls it.

limitabl-core / limitabl

  • AgentGuard opt-out + unbounded: the guard only ran when the client supplied a workflow id — omitting it bypassed runaway protection. preflight() now denies when a guard is configured but no key resolves. The facade namespaces the client workflow id under the server-resolved principal (sub/tenant/ip) so it is not purely client-controlled, and AgentGuard gains a lazy TTL sweep (evict workflows past maxDurationMs) so id rotation can't grow the map unbounded.
  • Singleton engine (facade): the module-level singleton ignored every config after the first (a stricter later mount reused the laxer engine). Now one engine per limitabl() invocation.

identifiabl-core

  • exp not required: jwtVerify accepted a token minted without exp — valid forever. Added requiredClaims:["exp"]. The factory now rejects an empty issuer or audience (empty audience disables audience binding; empty issuer makes the post-verify issuer check pass for iss:"").

explicabl

  • Audit events carried no identity: read res.locals.gatewayContext, which nothing sets — every line logged context: undefined. Now reads getGatewayContext() (ALS) with a captured-reference fallback, and warns once if absent.
  • Dead webhook: /webhooks/auth0 mounted the handler factory as middleware → requests hung forever. Now calls auth0LogsWebhook().
  • Aborted requests emitted no audit: only finish was hooked. Also hook close, with a fired-once guard so exactly one event emits.
  • Added @gatewaystack/request-context dependency + tsconfig project reference.

Repo hygiene (unblocks the above)

Removed 16 stale compiled .js/.d.ts artifacts committed inside src/ dirs. They were already gitignored but tracked, and several — validatabl-core/src/scopes.js, explicabl/src/index.js, limitabl/src/index.js — contained the exact pre-fix code this PR removes, shadowing the source on .js imports (this actually broke the first run of the new hasScope test until removed). Overlaps the #42 hygiene theme; done here because it directly contradicts these fixes.

Tests (30 new, all real behavior)

decision denies on absent input · hasScope("a.b") ∌ "aXb" · empty-conditions rejected · matches anchored · in strict · JWT round-trips (real jose + local JWKS) reject alg:none/expired/missing-exp/wrong-aud + factory guards · two limitabl mounts enforce different limits · agent guard denies without a key + TTL evicts · client-abort emits exactly one audit event · webhook route responds (no hang).

Full suite 189 green · npm run build clean.

Notes

Not merged/published — that's yours.

Security review of the OSS enforcement packages. Each finding is a
reputational/external-consumer correctness issue (prod's coding-agent hook
path uses a private engine), but the repo is the trust artifact.

validatabl-core:
- decision(): a configured inputSchema was skipped when input was undefined
  (GET, wrong content-type), so a schema meant to gate a route was a silent
  no-op. Now the schema always runs; missing input denies.
- hasScope(): built a RegExp from the raw scope string, so `tool.write`
  matched `toolXwrite` (`.` = any char) and metachars threw 500s. Replaced
  with an exact whitespace-delimited token match — no regex.
- policy widening: `matches` was unanchored (`search` allowed
  `unsafe-search-exec`) — bare patterns are now anchored ^(?:…)$ while an
  author's own anchors are honored; `in` used String() coercion (an array
  field stringified to "a,b" and slipped past an allow-list) — now strict
  string membership; empty-conditions rules matched every request (vacuous
  every()) — validatePolicySet() rejects them (loud) and applyPolicies calls
  it.

limitabl-core / limitabl:
- AgentGuard was client-opt-out: the guard only ran when the client supplied
  a workflow id, so omitting it bypassed runaway protection. preflight() now
  denies when a guard is configured but no key resolves. The facade namespaces
  the client workflow id under the server-resolved principal so it is not
  purely client-controlled. AgentGuard adds a lazy TTL sweep (evict workflows
  past maxDurationMs) so id rotation can't grow the map unbounded.
- The facade's module-level singleton engine ignored every config after the
  first (a stricter later mount reused the laxer engine). Build one engine per
  limitabl() invocation.

identifiabl-core:
- jwtVerify did not require `exp`, so a token minted without it never expired.
  Added requiredClaims:["exp"]. The factory now rejects an empty issuer or
  audience (empty audience disables audience binding; empty issuer makes the
  post-verify issuer check pass for iss:"").

explicabl:
- Audit events read res.locals.gatewayContext, which nothing sets — every
  line logged context: undefined. Now reads getGatewayContext() (ALS), with a
  captured-reference fallback, and warns once if absent.
- /webhooks/auth0 mounted the handler FACTORY as middleware, so requests hung
  forever and the webhook was dead. Call auth0LogsWebhook().
- Only `finish` was hooked, so aborted requests emitted no audit. Also hook
  `close`, with a fired-once guard so exactly one event emits.
- Added @gatewaystack/request-context dependency + tsconfig reference.

Repo hygiene (unblocks the above): removed 16 stale compiled .js/.d.ts
artifacts committed inside src/ dirs. These were already gitignored but
tracked, and several (validatabl-core/src/scopes.js, explicabl/src/index.js,
limitabl/src/index.js) contained the exact pre-fix code this PR removes —
they shadowed the source on .js imports and contradicted the fixes.

Tests: 30 new across the five packages — decision denies on absent input;
hasScope("a.b") ∌ "aXb"; empty-conditions rejected; matches anchored; in
strict; JWT round-trips reject alg:none/expired/missing-exp/wrong-aud + factory
guards; two limitabl mounts enforce different limits; agent guard denies
without a key + TTL evicts; client-abort emits exactly one audit event; webhook
route responds. Full suite 189 green; build clean.

Versions intentionally NOT bumped here — publish/version is David's step, and
explicabl also has a parallel bump in #37; coordinate at publish.
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.

Enforcement-core hardening: fail-open & policy-widening classes in validatabl/limitabl/identifiabl/explicabl

1 participant