Skip to content

Commit 770481a

Browse files
authored
rules: fail closed via viral CEL unknowns instead of dispatch gates (#633)
Replaces the dispatch-level truncation/unparseable pre-checks with value-level unknowns from cel-go partial evaluation, and makes runtime CEL evaluation errors fail closed. * `match.Matcher.Match` is now three-valued (Matched / NoMatch / Unevaluable). On a Truncated or Unparseable request the matcher marks the affected facet paths (the declared `TruncatablePaths` / `UnparseablePaths`) as CEL unknowns via a partial activation (`cel.PartialVars` + `OptPartialEval`); the unknowns propagate virally, NaN-style, and surface as Unevaluable, which the dispatcher turns into a synthesized deny attributed to the rule whose contract broke * More precise than the old reads-the-path gate: `&&`/`||` absorption lets a condition resolve honestly when its outcome doesn't depend on the unavailable value. `sql.verb == 'select' && sql.database == 'x'` on an unparseable query against database 'y' now cleanly no-matches (`unknown && false == false`) instead of synth-denying; the symmetric `unknown || true` fires as written * CEL eval errors no longer coerce to "no match" — a deny rule whose condition errors at runtime used to be silently skipped, failing open; it now synth-denies. The common case is selecting a `body_json` field the payload doesn't carry — or a missing map key like `k8s.params.stdin`. Guard optional fields with `has()` / `'key' in map`. `has()` cannot rescue a *truncated* field: the presence test itself is unknown * Agent-visible deny reasons carry only the rule name and the coarse cause (truncated / unparseable / evaluation error); the full detail — unknown facet paths or the raw CEL error text — goes to the gateway log. cel-go errors like `no such key: <field>` would otherwise let an agent probe which fields a rule inspects by varying payloads and reading deny reasons * New `runtime.MatchRequestFailClosed` backstop for SQL frontends (postgres, clickhouse_native): when a truncated or unparseable request matches no rule on an endpoint that declares rules, it denies instead of riding the implicit-allow default — "no rule matched" may just mean every condition resolved independent of the missing bytes. Rule-less endpoints keep pass-through; the HTTPS path keeps plain MatchRequest (rule-less LLM endpoints routinely forward >cap bodies) * `InspectsTruncatableFacet` stays as the compile-time laziness signal (`CompiledEndpoint.InspectsTruncatable` / ssh stdin buffering); `InspectsUnparseableFacet` is removed along with the dispatch gate. `OptPartialEval` and the unknown-pattern slices are built only for conditions that reference a truncatable / unparseable path, keeping cel-go's default attribute factory on the hot path * New tests pin the absorption semantics, the strict-error deny, the has() guard behavior on captured vs truncated bodies, unknown propagation through `==` / `in` / `exists()`, reason sanitization, and the SQL backstop; the `clawpatrol test` fixture corpus passes unchanged * Docs: rules.md gains a unified "Unevaluable conditions fail closed" section covering truncation, parser refusal, eval errors, and the SQL backstop; adds the missing ssh stdin row to the inspection-cap table; the body_json and k8s-no-interactive examples use guards; plugins.md documents that the optional-field zero-fill covers declared fields only
1 parent d2fb38c commit 770481a

28 files changed

Lines changed: 1012 additions & 354 deletions

cmd/clawpatrol/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1978,8 +1978,8 @@ func bufferHTTPBodyForMatch(req *http.Request, capBytes int) []byte {
19781978
// front of the original stream so upstream still receives the body
19791979
// byte-for-byte. truncated is true iff the body extended beyond
19801980
// maxHTTPMatchBody; callers stash this on match.Request.Truncated so
1981-
// the dispatcher can fail-close rules that read http.body /
1982-
// http.body_json.
1981+
// http.body / http.body_json become CEL unknowns and rules whose
1982+
// outcome depends on them fail-close.
19831983
func bufferHTTPBodyForMatchTruncated(req *http.Request, capBytes int) (body []byte, truncated bool) {
19841984
if req.Body == nil {
19851985
return nil, false

internal/config/compile_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,17 +92,17 @@ func TestCompile(t *testing.T) {
9292
}
9393
getReq := &match.Request{Family: "http", Method: "GET"}
9494
postReq := &match.Request{Family: "http", Method: "POST"}
95-
if !reads.Matcher.Match(getReq) {
95+
if reads.Matcher.Match(getReq).Result != match.Matched {
9696
t.Errorf("github-reads should match GET")
9797
}
98-
if reads.Matcher.Match(postReq) {
99-
t.Errorf("github-reads should NOT match POST")
98+
if got := reads.Matcher.Match(postReq).Result; got != match.NoMatch {
99+
t.Errorf("github-reads should NOT match POST, got %v", got)
100100
}
101-
if !writes.Matcher.Match(postReq) {
101+
if writes.Matcher.Match(postReq).Result != match.Matched {
102102
t.Errorf("github-writes should match POST")
103103
}
104-
if writes.Matcher.Match(getReq) {
105-
t.Errorf("github-writes should NOT match GET")
104+
if got := writes.Matcher.Match(getReq).Result; got != match.NoMatch {
105+
t.Errorf("github-writes should NOT match GET, got %v", got)
106106
}
107107

108108
// Outcomes wired correctly.

internal/config/extplugin/adapter.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -394,8 +394,8 @@ func handleEvaluate(ctx context.Context, ch *runtime.ConnHandle, ev *pb.Evaluate
394394
// Build a match.Request rich enough for the matcher AND for the
395395
// HITL prompt fields a human approver might render. Truncated
396396
// is set when at least one stream field hit its cap before
397-
// EOF — runtime.MatchRequest's fail-closed gate then auto-denies
398-
// any rule whose matcher reads a stream-typed field.
397+
// EOF — the matcher then marks stream-typed fields CEL-unknown
398+
// and any rule whose outcome depends on one is denied.
399399
var req *match.Request
400400
if pf != nil {
401401
req = &match.Request{

internal/config/extplugin/celenv.go

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,12 @@ import (
1919
//
2020
// streamFields names the FACET_STREAM fields on the facet. They're
2121
// passed to match.CompileCondition as truncatablePaths so the
22-
// dispatcher's existing fail-closed-on-truncation gate applies to
23-
// plugin facets too: when the gateway's pullStream had to cap a
24-
// stream short of EOF, the EvaluateAction handler sets
25-
// Request.Truncated and runtime.MatchRequest auto-denies any rule
26-
// whose CEL condition reads the stream-typed bytes.
22+
// fail-closed-on-truncation contract applies to plugin facets too:
23+
// when the gateway's pullStream had to cap a stream short of EOF,
24+
// the EvaluateAction handler sets Request.Truncated, the matcher
25+
// marks the stream-typed paths as CEL unknowns, and any rule whose
26+
// condition outcome depends on the capped bytes evaluates
27+
// Unevaluable — which runtime.MatchRequest turns into a deny.
2728
//
2829
// The returned matcher additionally implements SubFieldReferencer
2930
// so the gateway adapter can decide, per evaluation, which
@@ -81,21 +82,17 @@ type pluginMatcher struct {
8182
subFieldRefs map[string]bool
8283
}
8384

84-
func (m *pluginMatcher) Match(req *match.Request) bool { return m.inner.Match(req) }
85+
func (m *pluginMatcher) Match(req *match.Request) match.Decision { return m.inner.Match(req) }
8586

86-
// InspectsTruncatableFacet forwards the inner CEL matcher's
87-
// answer. The dispatcher uses it together with Request.Truncated
88-
// to fail-close any rule that reads a stream-typed field on a
89-
// request the gateway had to cap mid-pull.
87+
// InspectsTruncatableFacet forwards the inner CEL matcher's answer —
88+
// the compile-time laziness signal that tells the adapter whether
89+
// any rule needs a stream-typed field pulled in full. Verdicts on
90+
// truncated streams come from the inner matcher's Unevaluable result
91+
// (the stream fields are marked CEL-unknown on a Truncated request).
9092
func (m *pluginMatcher) InspectsTruncatableFacet() bool {
9193
return m.inner.InspectsTruncatableFacet()
9294
}
9395

94-
// InspectsUnparseableFacet always returns false: plugin facets have
95-
// no parser failure mode (see newPluginFacetMatcher's nil unparseable
96-
// path), so the Unparseable gate is structurally a no-op here.
97-
func (m *pluginMatcher) InspectsUnparseableFacet() bool { return false }
98-
9996
// References preserves whatever the inner matcher reports so the
10097
// gateway's existing body-buffering check (top-level identifier
10198
// reachability) keeps working.

internal/config/extplugin/facet.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ type pluginFacet struct {
4141
optionalFields map[string]bool
4242
// streamFields lists the FACET_STREAM field names. Passed to
4343
// the CEL compiler as truncatablePaths so the dispatcher's
44-
// fail-closed-on-truncation gate (req.Truncated +
45-
// matcher.InspectsTruncatableFacet) applies to plugin facets.
44+
// fail-closed-on-truncation contract (req.Truncated marks the
45+
// stream paths CEL-unknown) applies to plugin facets.
4646
streamFields []string
4747
}
4848

internal/config/facet/composition.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ import (
1313
// matcher: env options that declare its variable(s) and the Go types
1414
// behind them, an activation builder that populates its bindings on
1515
// the request's activation map, and the path lists CompileCondition
16-
// needs for case-normalization and truncation-fail-close.
16+
// needs for case-normalization and the unevaluable-fail-close
17+
// contract (truncated / unparseable values become CEL unknowns).
1718
//
1819
// Built-in facets return their own contribution via CELContributor.
1920
// Composition (a k8s_rule referencing http.method) is performed in
@@ -27,18 +28,19 @@ import (
2728
//
2829
// AddActivation writes the facet's bindings into act. It returns
2930
// false to refuse the match (e.g. wrong Meta type for the family);
30-
// when any contributor refuses, the composed matcher's Match returns
31-
// false without evaluating the CEL program.
31+
// when any contributor refuses, the composed matcher's Match reports
32+
// Unevaluable (fail closed) without evaluating the CEL program.
3233
type CELContrib struct {
3334
EnvOptions []cel.EnvOption
3435
AddActivation func(req *match.Request, act map[string]any) bool
3536
LowercasedPaths []string
3637
TruncatablePaths []string
3738
// UnparseablePaths lists the fields a wire frontend's parser
3839
// derives from the request bytes — when the parser refuses the
39-
// input, the frontend sets req.Unparseable=true and the
40-
// dispatcher auto-denies any rule whose CEL reads one of these
41-
// paths. Empty for facets with no parser-failure mode (http, k8s).
40+
// input, the frontend sets req.Unparseable=true and the matcher
41+
// marks these paths as CEL unknowns: any rule whose condition
42+
// outcome depends on one evaluates Unevaluable and is denied.
43+
// Empty for facets with no parser-failure mode (http, k8s).
4244
UnparseablePaths []string
4345
}
4446

0 commit comments

Comments
 (0)