security(policy): mistyped or unhonored policy rules load silently and disable row security #460

Area: policy — security (fail-open) · found via PR #457 review

Expected: a policy rule that the engine will not honor — because it was mistyped, or because it has no meaning for that operation — is rejected at config load, loudly. Row security is never silently disabled.

Actual: two independent gaps let a malformed rule load cleanly and produce no predicate at all, which on the row-filter path means the role reads the whole table.

1. A typo'd operator key silently disables row security

Every decoder is non-strict — internal/policy/store.go:153,185,210,214 and internal/api/policy.go:46,71 — so an unknown key is dropped rather than rejected. A Filter with all five operator pointers nil then contributes zero clauses (resolveFilters has one if per operator and no else), Evaluate leaves WhereClause empty (internal/policy/policy.go:192-196), and InjectPermissionFilters no-ops on an empty clause.

validateRolePerms (internal/policy/policy.go:517-523) checks only chsql.BindUnsafe(col) for filter columns — it never requires that an operator be set.

Verified end-to-end:

select:
  user:
    filter: { tenant_id: { eq: "{{ jwt.tenant_id }}" } }   # "eq", not "_eq"

→ Filter{Eq:nil, Neq:nil, Gt:nil, Lt:nil, In:nil}
→ Allowed=true, WhereClause=""
→ SELECT * FROM `clicks` LIMIT 10000        # entire table, every tenant
→ Validate() == nil, and POST /v1/admin/policy/validate returns {"valid":true}

A one-character typo disables RLS, and the validate endpoint actively confirms the policy is fine.

Note the asymmetry: Go's encoding/json matches field names case-insensitively, so _EQ and _Eq do work — but eq does not, and YAML is exact-match only. The format is lenient in a direction nobody needs and silent in the one that bites.

2. filter: under an insert: role entry is resolved, then ignored

internal/policy/policy.go:186-197 resolves filters for every operation, but internal/api/ingest.go reads only CheckClauses and the column rules. Nothing rejects the policy — in pointed contrast to the loud config-load rejection of _neq/_gt/_lt on check at internal/policy/policy.go:531. Same accept-but-ignore family as #224.

Scope

• Reject an all-nil Filter (and an all-nil check entry) in validateRolePerms.
• Reject filter: under an insert: role entry, matching the existing check operator rejection.
• Turn on strict decoding — yaml.Decoder.KnownFields(true) and DisallowUnknownFields — at the six decode sites above.

Migration risk (why this is not a drive-by)

Strict decoding is a breaking change to config parsing: any stored policy or bootstrap file with a stray or misspelled key currently loads fine and would start hard-failing, and Validate also runs on KV-loaded policies — so this can take a deployment down at boot. Consider warn-then-reject over one release, and decide explicitly whether an already-stored bad policy should fail boot or load with a loud warning.

Related

• fix(policy): fail closed on unresolvable row-filter claims #457 added a template-grammar check to validateRolePerms for the same reason (an unhonored rule must not load silently) — that is the first instance of this pattern; these are the rest.
• feat(policy): validate/fmt tooling + JSON Schema for policy & config files #243 (JSON Schema + offline policy validate CLI) would catch case 1 at authoring time, in-editor, before it ever reaches the server. Natural companion.
• Policy filter/check: _in accepted but never enforced; check honors only _eq #224 — the original accept-but-ignore fail-open this shares a family with.

Found during the PR #457 review (fail-closed row-filter claims); verified by execution, not code-trace alone.

Update (2026-09-01)

Resolved across #508 and PR #541 — item by item against the Scope list:

• Reject an all-nil Filter (and an all-nil check entry): landed in PR fix(policy): reject no-op rules, drop the policy HTTP surface #541 (validateRolePerms rejects an operator-less filter or check entry). The explicit spelling "tenant_id": {} — which strict decoding cannot catch, being syntactically innocent — no longer adopts.
• Reject filter: under an insert: role entry: landed in PR fix(policy): reject no-op rules, drop the policy HTTP surface #541, matching the existing loud check _neq/_gt/_lt rejection.
• Strict decoding at the six decode sites: done by feat(settings): hot-reload the settings directory at boot and runtime #508 for the settings directory (the runtime authority; the old store/KV sites are deleted), and completed by PR fix(policy): reject no-op rules, drop the policy HTTP surface #541 removing POST /v1/ops/policy/validate — the last lenient decoder — outright (the PR drops the policy's whole HTTP surface, GET /v1/ops/policy included).
• Migration risk: moot; the settings directory shipped new and unreleased, and no seed, compose, or fixture policy carries any rejected shape.

Because #508 funnels every adoption (boot, watch, SIGHUP, reload, wavehouse validate) through the one policy.Validate path, the new rejections apply on every surface at once.

Note: #508's "Closes" list auto-closed only #229 (GitHub binds one issue per keyword) — this issue stayed open by accident, correctly so given the items above were still outstanding.