Skip to content

security(policy): define insert-check behavior when a claim template can't be resolved #463

Description

@EricAndrechek

Area: policy · ingest — security (write/read asymmetry) · found via PR #457 review

Expected: the insert-check path has a defined, documented answer for "the policy template references a claim this token doesn't carry," and that answer is consistent with what the read path does.

Actual: the two paths disagree, and after #457 the disagreement produces data that can be written but never read.

The asymmetry

Evaluate resolves a check _eq template and keeps the rendered value even when the claim is unresolvable (internal/policy/policy.go:205-210 — the error from resolveTemplate is deliberately discarded). That empty string then becomes the required value, which internal/api/ingest.go:397-400 auto-injects into any record that omits the column.

#457 made the row-filter path fail closed for the same input. The result:

Before #457 After #457
Claim-less token inserts row written, '' auto-injected into tenant_id unchanged — row written, '' auto-injected
Same token reads back filter was tenant_id = '' → sees its own rows filter is 1 = 0sees nothing, ever

So a validly-signed token missing the claim (the #385 scenario: service tokens, mixed IdP audiences) still successfully writes rows, and those rows are now unreachable to it. They also pool under a shared '' tenant value, visible to any token that carries tenant_id: "" explicitly — which #457 documented as binding normally.

Failing closed on read is the right call; this issue is about the write side not having made a matching decision.

Why #371 doesn't cover this

#457's code comment and docs/src/content/docs/access-control.mdx both point at #371 for check-path semantics. #371's scope is three boxes — add _neq to check, split PolicyCheck from PolicyFilter in the SDK, and scope typed _gt/_lt — all of which are about which operators check honors. None addresses what happens when the claim is unresolvable, and that question applies to _eq, the one operator #371 doesn't touch. #457 corrects the pointer to reference this issue instead.

Scope

Decide and implement the check-path answer. The options are genuinely different, so this wants a decision before code:

  • Fail closed on the write too — reject the insert (403 check failed) when the claim is unresolvable, instead of auto-injecting ''. Symmetric with the read path and removes the unreachable-data case. Breaking: roles that insert successfully today start getting 403s.
  • Keep auto-inject, make the sentinel explicit — keep today's behavior but stop using '', which collides with a legitimate empty-string claim value.
  • Keep today's behavior, document it as intended — cheapest; leaves the write/read asymmetry as a known, recorded property.

A related sub-case, also undecided: a check template whose claim path is outside the {{ jwt.… }} grammar (hyphenated or namespaced OIDC claims) makes the literal {{ … }} text the required value, which is then stamped into every row omitting the column. #457 adds config-load validation rejecting such templates, which closes that door — but the underlying "what is the required value when there is no value" question is this issue's.

Related


Found during the PR #457 review; the read/write divergence is a consequence of that PR, the underlying check-path behavior predates it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/ingestIngest pipeline (Bento, batching, DLQ)area/policyAccess control policies (Hasura-style)bugSomething isn't workingsecuritySecurity-sensitive issue or fix

    Type

    No type

    Projects

    Status
    Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions