diff --git a/.agents/AGENTS.md b/.agents/AGENTS.md index 313d631..530bebc 100644 --- a/.agents/AGENTS.md +++ b/.agents/AGENTS.md @@ -6,13 +6,19 @@ you came to do. ## What this project is -`request-validator` is a generic Envoy / Istio **ext-authz HTTP** service. -Envoy forwards an incoming request to us, we say `allow` or `deny`, and -Envoy enforces. The decision is driven by a **CEL-based** YAML policy. +`request-validator` is a generic Envoy / Istio policy service with two +engines driven by one **CEL-based** YAML policy: + +- **extAuthz** (HTTP ext-authz): Envoy forwards a request, we say `allow` + or `deny`, Envoy enforces. +- **extProc** (gRPC ext_proc): we inspect and mutate live traffic + (request or response headers and body), or short-circuit and serve our + own response. It was built to cover the cases plain Istio `AuthorizationPolicy` cannot (inspecting request bodies, combining CIDRs with JSON contents, validating -OAuth `redirect_uris`, etc.). +OAuth `redirect_uris`, rewriting an untrusted DCR redirect into a warning +page, etc.). ## When you arrive, read this first @@ -29,7 +35,7 @@ Only after that, drill into: - **`.agents/DECISIONS.md`** - _why_ the project looks the way it does (CEL over a custom DSL, fail-closed semantics, in-process feed fetching, - HTTP ext-authz over gRPC for now, etc.). + the two-engine split, the mutation and directResponse model, etc.). - **`.agents/CODE_CONVENTIONS.md`** - house style for Go in this repo. - **`.agents/TESTING.md`** - how to run the test suite and the E2E recipe. - **`.agents/OPERATIONS.md`** - deploy notes, observability, troubleshooting. @@ -44,7 +50,8 @@ Only after that, drill into: │ ├── celenv/ CEL environment + custom functions │ ├── configwatch/ fsnotify wrapper for policy hot-reload │ ├── facts/ facts registry (inline/file/url sources) -│ ├── httpserver/ ext-authz HTTP endpoint + metrics +│ ├── httpserver/ extAuthz HTTP endpoint + metrics +│ ├── grpcserver/ extProc gRPC endpoint (Envoy ext_proc) │ ├── jsonpath/ tiny JSONPath subset used by the engine │ ├── log/ slog wrapper (json | console handlers) │ └── policy/ policy types, parser, evaluator @@ -56,17 +63,18 @@ Only after that, drill into: └── .agents/ this directory ``` -## Common tasks → where to start - -| You want to... | Read | Touch | -| ------------------------------------------------ | ------------------------------------------------------------ | ------------------------------------------- | -| Add a new CEL function (e.g. `b64Url`) | `ARCHITECTURE.md` → "CEL environment", `CODE_CONVENTIONS.md` | `internal/celenv/.go` | -| Add a new source method for `facts:` | `ARCHITECTURE.md` → "Facts lifecycle" | `internal/facts/facts.go` | -| Tweak the access log shape or redaction rules | `POLICY_DSL.md` → "logging", `ARCHITECTURE.md` → "Logging" | `internal/httpserver/access.go` + `policy/` | -| Add a new top-level YAML section | `POLICY_DSL.md`, `ARCHITECTURE.md` → "Config types" | `internal/policy/policy.go` | -| Speed up the request hot path | `ARCHITECTURE.md` → "Request lifecycle" | `internal/httpserver/server.go` | -| Change deploy/observability behaviour | `OPERATIONS.md` | Dockerfile, `.github/workflows/`, README | -| Understand a past decision (e.g. "why no gRPC?") | `DECISIONS.md` | - | +## Common tasks: where to start + +| You want to... | Read | Touch | +| ------------------------------------------------ | ------------------------------------------------------------- | ------------------------------------------- | +| Add a new CEL function (e.g. `b64Url`) | `ARCHITECTURE.md` ("CEL environment"), `CODE_CONVENTIONS.md` | `internal/celenv/.go` | +| Add a new source method for `facts:` | `ARCHITECTURE.md` ("Facts lifecycle") | `internal/facts/facts.go` | +| Tweak the access log shape or redaction rules | `POLICY_DSL.md` ("logging"), `ARCHITECTURE.md` ("Logging") | `internal/httpserver/access.go` + `policy/` | +| Add a new top-level YAML section | `POLICY_DSL.md`, `ARCHITECTURE.md` ("Config types") | `internal/policy/policy.go` | +| Add or change an extProc mutation op | `POLICY_DSL.md` ("Mutation ops"), `DECISIONS.md` (D-019/D-023) | `internal/policy/` + `internal/grpcserver/` | +| Speed up the request hot path | `ARCHITECTURE.md` ("Request lifecycle") | `internal/httpserver/server.go` | +| Change deploy/observability behaviour | `OPERATIONS.md` | Dockerfile, `.github/workflows/`, README | +| Understand a past decision | `DECISIONS.md` | - | ## Hard rules - do not break diff --git a/.agents/ARCHITECTURE.md b/.agents/ARCHITECTURE.md index d1a0c51..56f21f2 100644 --- a/.agents/ARCHITECTURE.md +++ b/.agents/ARCHITECTURE.md @@ -12,21 +12,23 @@ internal/ celenv/ CEL environment, custom functions, program cache jsonpath/ tiny JSONPath subset (used by `jsonPath` CEL fn) facts/ facts registry: inline values, file reads, URL fetchers - policy/ config types, YAML parser, evaluator + policy/ config types, YAML parser, evaluators (extAuthz + extProc) configwatch/ fsnotify wrapper, debounce, k8s ConfigMap-aware - httpserver/ /healthz, /readyz, /metrics, ext-authz endpoint + httpserver/ /healthz, /readyz, /metrics, extAuthz endpoint + grpcserver/ extProc endpoint (Envoy ext_proc gRPC stream) ``` The dependency graph is acyclic and one-directional: ``` -cmd ──► policy ──► facts - │ │ - ├──► celenv ──► jsonpath - │ -cmd ──► httpserver ──► policy -cmd ──► configwatch -cmd ──► log (everyone else also imports log) +cmd --> policy --> facts + | | + +--> celenv --> jsonpath + | +cmd --> httpserver --> policy +cmd --> grpcserver --> policy +cmd --> configwatch +cmd --> log (everyone else also imports log) ``` `log` is the only package every other one depends on. It must stay @@ -38,10 +40,10 @@ dependency-free of the rest. ``` Config{ - Defaults Defaults // action, denyStatus, denyBody, maxBodyBytes, allowOnError + Defaults Defaults // per-engine: Defaults.ExtAuthz + Defaults.ExtProc + global DryRun Logging Logging // level, format, exclude/redact headers, etc. Facts []facts.Spec // declared facts - Groups []Group // ordered list of rule buckets + Groups []Group // ordered list of rule buckets, each bound to one engine // not in YAML, set during LoadBytes: env *celenv.Env // shared CEL env + program cache @@ -49,9 +51,12 @@ Config{ } ``` -A `Group` carries its compiled `matchProg cel.Program`, and each `Rule` -carries its own. Compilation happens once in `LoadBytes()`; the request -path only executes already-compiled programs. +A `Group` carries `parameters` (engine, mode, phase) and its compiled +`matchProg cel.Program`. Each `Rule` carries its own `matchProg`, plus +either a `Validation` (extAuthz) or a list of `Mutation` whose CEL +expressions are also compiled (value/code/headers/body). Compilation +happens once in `LoadBytes()`; the request path only executes +already-compiled programs. ## Request lifecycle @@ -72,14 +77,14 @@ path only executes already-compiled programs. ┌────────────┴────────────┐ │ for each Group, in order│ └────────────┬────────────┘ - │ 6. group.matchProg → bool + │ 6. group.matchProg to bool │ (skip silently if false) ▼ ┌──────────────────────────┐ │ Group.Mode == firstMatch │ every rule: - │ or == all │ rule.matchProg → bool - └──────────────┬───────────┘ + action inheritance - ▼ + dryRun + fallthrough + │ or == all │ rule.matchProg to bool + └──────────────┬───────────┘ + validation.action + ▼ + dryRun (no fallthrough) ┌──────────┐ │ Decision │ │ {Allowed,│ @@ -96,12 +101,44 @@ path only executes already-compiled programs. There is exactly **one** access-log record per request, level `INFO` for allow / `WARN` for deny. The CEL programs were compiled at policy load, so the only per-request cost is body read + map build + a few CEL calls. +Only groups whose `parameters.engine` is `extAuthz` are evaluated here; +`firstMatch` lets the first matching rule decide, `matchAll` requires +every rule to match or denies. There is no action inheritance and no +fallthrough. + +## Response lifecycle (extProc, gRPC) + +`grpcserver` implements Envoy's `ext_proc` bidirectional stream. Each +stream message maps to a phase (`requestHeaders`, `requestBody`, +`responseHeaders`, `responseBody`); the server keeps per-stream state +(the request, then the response) and calls `policy.EvaluateProc(phase, +req, resp)`, which walks the extProc groups bound to that phase +(`firstMatch` or `applyAll`) and returns the resolved mutations (CEL +values already evaluated). The server then: + +- if a `directResponse` is applicable, emits an Envoy `ImmediateResponse` + (status + headers + body) and ignores the rest (short-circuit); +- otherwise builds a `CommonResponse` with the header/body mutations; +- under dry-run (global or per-rule), responds CONTINUE while logging what + it would have done; +- on a body phase, enforces `extProc.maxBodyBytes` with + `onBodyOverflow: skip | fail`. + +Live CEL variables follow the phase: `request`/`facts` in request phases, +plus `response` in response phases. ## CEL environment (`internal/celenv`) -Built once per policy load, in `celenv.New()`: +Built once per policy load, in `celenv.New()`, as two scoped environments: -- Variables declared: `request` (dyn) and `facts` (dyn). +- `ScopeRequest` declares `request` (dyn) and `facts` (dyn). +- `ScopeResponse` declares `request`, `response` (dyn) and `facts`. + An expression that references `response` in a request scope fails to + compile, which is how the per-phase variable contract is enforced. +- Compilation is typed: `Compile` (bool, for `match`), `CompileString` + (header/body values), `CompileInt` (`setStatus` code), `CompileStringMap` + (`directResponse.headers`, `map`). Output type is checked + at load. Their `Eval*` counterparts re-check the type at runtime. - Standard library + these extensions enabled: `ext.Strings()`, `ext.Encoders()`, `ext.Lists()`, `ext.Sets()`, `ext.Math()`, `ext.Bindings()`. @@ -136,17 +173,17 @@ policy.LoadFile │ ▼ policy.LoadBytes - ├─ yaml.Unmarshal → Config{Defaults, Logging, Facts, Groups} + ├─ yaml.Unmarshal to Config{Defaults, Logging, Facts, Groups} ├─ applyDefaults ├─ validate - ├─ celenv.New ← every Compile() is cached - ├─ facts.New(Facts) ← builds Registry, value entries already populated - └─ compile ← turn match strings into cel.Program + ├─ celenv.New (every Compile() is cached) + ├─ facts.New(Facts) (builds Registry, value entries already populated) + └─ compile (turn match/mutation strings into cel.Program) cfg.Start(ctx) └─ for each fact: - file → os.ReadFile → store as string - url → http GET → store as string + spawn goroutine + file os.ReadFile store as string + url http GET store as string + spawn goroutine with time.Ticker(interval) ``` @@ -205,17 +242,24 @@ masked. The redaction policy: a value of length `< 2 * redactReveal` is fully masked; otherwise the first `redactReveal` characters are shown and the rest replaced with `*`. -## HTTP server endpoints +## Servers and endpoints + +The HTTP server (`httpserver`, default `:8080`) serves extAuthz plus +operational endpoints: | Path | Purpose | | ---------- | ----------------------------------------------------------------------- | -| `/` | ext-authz check. Envoy POSTs the original request here. | +| `/` | extAuthz check. Envoy POSTs the original request here. | | `/healthz` | always 200 once the process is up. | | `/readyz` | 200 only after the first policy is installed (used as readiness probe). | | `/metrics` | Prometheus text format. Counters per (rule, outcome, dry_run). | `/` accepts any method and path; it inspects whatever Envoy forwarded. +The gRPC server (`grpcserver`, default `:9090`) serves the Envoy ext_proc +`ExternalProcessor` service for the extProc engine. Both servers share the +same `*policy.Config` pointer and the same hot-reload path in `cmd`. + ## Build & ship - `Dockerfile` produces a `gcr.io/distroless/static:nonroot` image with diff --git a/.agents/DECISIONS.md b/.agents/DECISIONS.md index e26a7a4..8faa764 100644 --- a/.agents/DECISIONS.md +++ b/.agents/DECISIONS.md @@ -283,3 +283,311 @@ suppression check belongs there. `request_validator_rule_decisions_total{outcome="deny",dry_run="true"}`. One decision metric per request, with `dry_run` reflecting whether enforcement was suppressed. + + +## D-015 - Two engines: ext_authz (decide) and ext_proc (mutate) + +**Context.** The service was a pure ext-authz: read a request, return +allow/deny. A new need appeared: intercept the response Keycloak emits +during DCR with remote MCPs and, for untrusted redirect targets, rewrite +the `Location` header so the user lands on an interstitial warning page +instead of a risky destination. The same applies more broadly to +inspecting and rewriting traffic, not just admitting it. Doing this with +an Envoy Lua filter that embeds HTML in the data plane is a mess. + +**Decision.** Add a second engine, `extProc`, served over Envoy's +`ext_proc` gRPC API, living next to the existing HTTP ext-authz. One +binary, two listeners, one shared policy and hot-reload. This supersedes +the "HTTP only, no gRPC" stance of D-008 by adding gRPC for a different +filter, not by replacing the HTTP ext-authz. + +**Reasoning.** + +- ext_authz is a binary gatekeeper (pass/deny). ext_proc is the only + Envoy hook that can mutate headers, body and status of live traffic + in both directions. The redirect-rewrite case needs mutation. +- Keeping both in one process means one policy language, one facts + registry, one reload path. Operators learn one tool. +- The HTML for the interstitial is loaded as a `file`/`url` fact and + injected via a `setBody` mutation. The HTML never lives in code or in + the data plane. + +**Consequences.** The binary now links the Envoy ext_proc protos and a +gRPC server. The policy grammar grows an explicit engine selector and a +mutation vocabulary (D-016..D-021). + +## D-016 - Explicit `engine` per group, inside `parameters` + +**Context.** A group must declare whether it programs the ext_authz +filter or the ext_proc filter. They are different Envoy filters with +different contracts. Hiding which one a group targets (or inferring it +from the rule contents) is exactly the confusion to avoid. + +**Decision.** Every group carries a `parameters` block. `parameters.engine` +is `extAuthz | extProc`, required, no default. Engine-specific knobs live +inside `parameters` too: `mode` (both engines) and `phase` (extProc only). + +```yaml +groups: + - name: ... + parameters: + engine: extAuthz | extProc + mode: ... + phase: ... # extProc only + match: ... # group guard, optional + rules: [...] +``` + +**Reasoning.** `engine` is the vocabulary the platform team already speaks +(they read `EnvoyFilter`/`WasmPlugin` daily). An explicit discriminator +beats magic. `parameters` separates "engine wiring" from policy logic +(`match`, `rules`). + +**Consequences.** The loader validates engine-specific coherence (D-017, +D-018, D-021). A group is unambiguously routed to the HTTP server +(extAuthz) or the gRPC server (extProc) by `engine` alone. + +## D-017 - Mode names are engine-specific to avoid a colliding `all` + +**Context.** Both engines compose rules, but the composition means +different things. extAuthz composes verdicts; extProc composes mutations. +Reusing `mode: all` for both would make one keyword mean "every rule must +hold (logical AND, else deny)" in one engine and "apply the mutations of +every matching rule" in the other. + +**Decision.** Distinct names per engine: + +- extAuthz: `mode: firstMatch | matchAll` +- extProc: `mode: firstMatch | applyAll` + +`firstMatch` means the same on both (first rule whose `match` is true +decides/applies, then stop). `matchAll` is the logical-AND verdict mode +(every rule must match or the group denies). `applyAll` accumulates the +mutations of every matching rule, in declaration order. + +**Reasoning.** Same word, same meaning; different behaviour, different +word. The verb in the name tells the user what the group does. + +**Consequences.** The loader rejects any mode not valid for the group's +engine. `firstMatch` is the only shared value. + +## D-018 - Homogeneous rule body: `validation` (singular) XOR `mutations` + +**Context.** A rule must read clearly as either "this decides" or "this +mutates". The two engines never mix in v1. + +**Decision.** + +- A rule's `match` is a **mandatory** CEL bool. There is no implicit + `true` default any more, and `fallthrough` is removed. In `firstMatch` + the first rule whose `match` is true wins; if none match, the group + produces no verdict and evaluation falls through to the next group / + `defaults`. +- An extAuthz rule carries `validation: { action: allow | deny }`. It is + a singular object, not a list: a rule produces exactly one verdict. + Group-to-rule action inheritance (old D-004) is removed; each rule + states its own verdict. +- An extProc rule carries `mutations: [ ... ]`, a real list (a rule may + legitimately apply several mutations). An empty `mutations: []` is a + valid "matched but change nothing" that still stops a `firstMatch` + group. + +**Reasoning.** `validation`/`mutations` name the intent. Mandatory +`match` and no inheritance remove the invisible-default footguns that +D-003/D-004 still allowed. One rule = one condition = one effect (verdict +or a set of mutations); several conditional effects are several rules. +This supersedes the default-`true` of D-003, the inheritance of D-004 and +the `fallthrough` of D-005. + +**Consequences.** Examples get slightly longer (explicit `match: 'true'` +for a fallback rule, explicit `action` on every extAuthz rule) in +exchange for zero hidden behaviour. + +## D-019 - Mutation ops: explicit `op` discriminator, fixed catalogue + +**Context.** Each item in `mutations` needs a clear, validatable shape. + +**Decision.** Each mutation is an object with an explicit `op` field and +its operands. v1 catalogue: + +| op | fields | legal phases | semantics | +| -------------- | ------------ | ----------------------------- | ------------------------------------------ | +| `setHeader` | name, value | all | upsert: overwrite if present, else create | +| `appendHeader` | name, value | all | add a value, keep existing ones | +| `removeHeader` | name | all | delete the header; no-op if absent | +| `setBody` | value | requestBody, responseBody | replace the body; recomputes Content-Length| +| `setStatus` | code | responseHeaders, responseBody | set the response status code | + +`value`/`code` are CEL expressions (string for header/body, int for +status). On `applyAll`, when two matching rules write the same header the +last one in declaration order wins. + +**Reasoning.** Explicit `op` is a clean switch in Go, easy to validate, +no "map with a single key" trick. `setHeader` as upsert is the least +surprising default. `removeHeader` is idempotent. `setBody` owns +Content-Length recomputation because making the user maintain it by hand +is a trap; Content-Type stays the user's responsibility. + +**Consequences.** `setStatus`/`setBody` legality depends on the group's +`phase`; the loader enforces it. Body-rewriting phases require Envoy to +buffer the body (D-021). + +## D-020 - CEL `response` variable; typed mutation expressions + +**Context.** Response-phase policies need to see the upstream response. +Mutation expressions return strings/ints, not booleans. + +**Decision.** + +- Add a CEL variable `response` (headers/header, status, body.json/yaml/raw), + shaped like `request`. Live variables depend on the phase: + - request phases (`requestHeaders`, `requestBody`): `request`, `facts`. + - response phases (`responseHeaders`, `responseBody`): `request`, + `response`, `facts`. +- `match` expressions are compiled and checked as bool (as today). + Mutation `value`/`code` expressions are a second kind of program, + checked for string/int output type at load time. A rule that references + `response` in a request phase fails to compile, not at runtime. + +**Reasoning.** Deriving live variables from the phase makes the +variable contract impossible to violate silently. Output-type checking at +load turns a class of runtime errors into load errors. + +**Consequences.** `celenv.Compile` gains a typed variant (or a parameter) +for non-bool expressions. The env is built per-phase or the variables are +registered and the loader rejects out-of-phase references. + +## D-021 - Per-engine `defaults`; ext_authz body overflow is fixed deny + +**Context.** `defaults.action` and `defaults.denyStatus` only make sense +for ext_authz; ext_proc does not "deny", it mutates. A flat `defaults` +mixing both engines breeds orphan fields. Body buffering limits matter to +both engines but can want different values. + +**Decision.** Split defaults per engine; each block is self-contained: + +```yaml +defaults: + extAuthz: + action: deny + denyStatus: 403 + denyBody: "Forbidden" + allowOnError: false + maxBodyBytes: 1MiB + extProc: + maxBodyBytes: 1MiB + onBodyOverflow: skip | fail + dryRun: false # global, both engines (D-022) +``` + +- `onBodyOverflow` exists **only** for extProc (`skip`: don't mutate, let + the original body pass; `fail`: immediate-response, fail-closed). There + is no `truncate`. +- ext_authz body overflow is **not configurable**: it is a fixed + fail-closed deny. A gatekeeper that cannot read the whole request does + not admit it. (This changes the old behaviour of truncating and + evaluating with a partial body.) + +**Reasoning.** Per-engine blocks make every knob's scope obvious and let +the gatekeeper and the surgeon tune body limits independently. Truncate +is dropped everywhere because a partial body yields wrong matches and +half-rewritten bodies. + +**Consequences.** Flat `defaults.action`/`maxBodyBytes`/etc. no longer +exist; they move under `defaults.extAuthz`. This is a breaking config +change with no migration shim (intentional). + +## D-022 - Dry-run stays global and suppresses every enforcing path + +**Context.** Operators want to shadow the whole policy (both engines) +before it can affect traffic. + +**Decision.** `defaults.dryRun` stays a single global switch (no +per-engine split for now). Per-rule `dryRun` also stays. When dry-run is +on, no path touches traffic: + +- extAuthz: a `deny` verdict is logged/metered as "would deny" but + returns 200. A body-overflow that would deny also passes with 200. +- extProc: mutations are computed and logged ("would setHeader ...") but + the gRPC stream responds CONTINUE with no mutation. An overflow that + would `fail` is logged as "would fail" and passes. + +**Reasoning.** Shadow mode must be observe-only on every branch, not just +the happy path, or the preview lies. + +**Consequences.** Both servers route their enforcement through a single +`effectiveDry` check before emitting the verdict/mutation, mirroring the +existing httpserver pattern (D-014). + + +## D-023 - `directResponse` op: short-circuit and serve a response + +**Context.** The flagship case (intercept Keycloak's DCR `302` to an +untrusted destination and show the user a warning page) is not a header +mutation: it is replacing the upstream response entirely with our own. +Expressing it as `setStatus` + `setHeader` + `setBody` has two problems: +`setBody` is illegal in `responseHeaders` (D-019) and the `responseBody` +phase never fires for a bodiless `302`, so the only phase that runs is +`responseHeaders`, where the incremental mutation ops cannot build a full +response. It is also three coupled mutations that only make sense +together. + +**Decision.** A dedicated extProc op, `directResponse`, that maps to +Envoy's `ImmediateResponse`: it discards the upstream response and serves +the one the rule builds. Shape: + +```yaml +- op: directResponse + status: 200 # int literal + headers: | # CEL expression evaluating to map + {"content-type": "text/html"} + body: | # CEL expression evaluating to string + facts.interstitialHtml.replace('__TARGET_B64__', base64.encode(response.header['location'])) +``` + +- `status` is an int literal (a CEL-computed status is YAGNI and less + readable). +- `headers` is a CEL expression returning `map`. This is + the single mechanism for every header need: a map literal for fixed + values, `response.header` to carry over all of the original headers + ("give me all of them"), and filter/merge expressions for any subset or + override. There is no inherit-by-default; the response is built from + scratch, which is faithful to the op's name. Header values are single + strings (no multi-value output); the rare multi-value case is out of + scope for v1. +- `body` is a CEL expression returning a string. + +`directResponse` is legal in `responseHeaders` and `responseBody`. In +`responseHeaders` it works even when upstream sent no body, which is the +`302` case. It is self-contained: a rule carrying `directResponse` must +not also carry any incremental op (setHeader/appendHeader/removeHeader/ +setBody/setStatus). The loader rejects the mix. In `applyAll`, the first +rule that fires a `directResponse` wins and stops the group (you cannot +short-circuit twice). + +**Reasoning.** What the user wants is "abort and serve this", which is an +ImmediateResponse, not a mutation. Treating `headers` as data built by a +CEL expression (like `body` already is) keeps one paradigm across the DSL +and removes the earlier dead ends (a `headers` map of CEL strings forced +`"'text/html'"` per entry; a list with value/expr/from/all/remove verbs +was a sub-language nobody wanted). "Give me all the original headers" is +just the expression `response.header`. + +**Consequences.** + +- celenv gains a fourth compiled kind: `CompileStringMap` / + `EvalStringMap` (output `map`), same pattern as the bool/ + string/int variants. Output type is checked at load; a runtime value + that is not a string map is fail-safe (the directResponse is not + emitted). +- The resolved-mutation model grows a variant carrying status + headers + + body. The gRPC server maps it to `ImmediateResponse{Status, Headers, + Body}` instead of a `CommonResponse` mutation. +- Under global or per-rule dry-run, `directResponse` is computed and + logged ("would directRespond") but not served; the stream responds + CONTINUE. +- Security note for the interstitial recipe: the original `Location` is + attacker-controlled (untrusted DCR client). It must be encoded before + being placed in the page (base64 in CEL, decoded by the button's JS) to + avoid HTML injection. The example uses `base64.encode(...)` and a + `__TARGET_B64__` placeholder. diff --git a/.agents/GLOSSARY.md b/.agents/GLOSSARY.md index 2afc7f4..3542e94 100644 --- a/.agents/GLOSSARY.md +++ b/.agents/GLOSSARY.md @@ -11,43 +11,62 @@ as a sample; in production it ships as a Kubernetes ConfigMap mounted into the pod. **Engine** -The in-memory runtime that turns a policy + a request into a verdict. -Lives in `internal/policy/policy.go`. +Which Envoy filter a group programs, set in `parameters.engine`: +`extAuthz` (HTTP ext-authz, produces allow/deny) or `extProc` (gRPC +ext_proc, mutates traffic or serves a direct response). The in-memory +runtime lives in `internal/policy` (`Evaluate` for extAuthz, `EvaluateProc` +for extProc). **Group** -A named bucket of rules in the policy. Has a `mode` and an optional -`match` (its scope). Groups are evaluated in declared order. +A named bucket of rules in the policy. Carries `parameters` (engine, mode, +and for extProc a phase) and an optional `match` (its scope). Groups are +evaluated in declared order. **Rule** -A single CEL boolean expression with an `action`. The leaf of a -decision. +The leaf of a decision. Always has a mandatory `match` (CEL boolean) plus +either a `validation` (extAuthz verdict) or a list of `mutations` +(extProc). **Mode** +How a group composes its rules. Engine-specific: -- `firstMatch` - the first rule whose `match` is true decides. -- `all` - every rule must hold; one failure produces the opposite - verdict. +- extAuthz: `firstMatch` (first rule whose `match` is true decides) or + `matchAll` (every rule must match, else deny). +- extProc: `firstMatch` (first matching rule applies its mutations and + stops) or `applyAll` (every matching rule applies its mutations, in + order, last write wins per header). **Action** -The verdict a rule (or group) produces when it matches: `allow` or -`deny`. A rule inherits the group's action when it doesn't declare one. +The verdict an extAuthz rule produces, in `validation.action`: `allow` or +`deny`. Each rule states its own; there is no inheritance. **Match** -The CEL boolean expression at the group or rule level. The group's -`match` is the scope; the rule's `match` is the decision condition. +The CEL boolean expression at the group or rule level. The group's `match` +is the scope; the rule's `match` is the condition. The rule `match` is +mandatory. -**Fallthrough** -What to do in a `firstMatch` group when a rule's `match` is false: -`next` (try the next rule, the default), `allow` or `deny` -(short-circuit). +**Phase** (extProc only) +Which ext_proc hook a group runs on: `requestHeaders`, `requestBody`, +`responseHeaders`, `responseBody`. Determines the live CEL variables +(`response` is only available in response phases). + +**Mutation** +An extProc traffic change in a rule's `mutations` list. Incremental ops: +`setHeader`, `appendHeader`, `removeHeader`, `setBody`, `setStatus`. + +**directResponse** +The extProc op that discards the upstream response and serves one the +rule builds (`status`, `headers` as a CEL map, `body` as a CEL string), +mapping to Envoy's ImmediateResponse. Exclusive in its rule. **DryRun** -A rule with `dryRun: true` is evaluated and logged but never actually -denies. Used for shadow-testing future tightenings. +A rule with `dryRun: true`, or the global `defaults.dryRun`, is evaluated +and logged but never enforced: extAuthz passes 200, extProc responds +CONTINUE without mutations. **Default action** -What happens when no group decides. Configured under -`defaults.action`; defaults to `deny` (fail-closed). +The extAuthz verdict when no group decides. Configured under +`defaults.extAuthz.action`; defaults to `deny` (fail-closed). ## Facts subsystem @@ -99,13 +118,23 @@ shapes; whichever succeeds populates the corresponding fields. **ext-authz** Envoy's "External Authorization" filter. We implement the HTTP variant -(`envoyExtAuthzHttp`). Envoy POSTs the original request to us, we -return 200 or a non-2xx with the configured body. +(`envoyExtAuthzHttp`). Envoy POSTs the original request to us, we return +200 or a non-2xx with the configured body. Served by `internal/httpserver`. + +**ext_proc** +Envoy's "External Processing" filter (gRPC). Envoy streams the request and +response phases to us; we reply with mutations, a direct response, or +CONTINUE. Served by `internal/grpcserver`. **Decision** -The Go struct (`policy.Decision`) carrying the verdict, the winning -rule name, a short `reason`, and the `dry_run` flag. Becomes the -response status + headers. +The Go struct (`policy.Decision`) carrying the extAuthz verdict, the +winning rule name, a short `reason`, and the `dry_run` flag. Becomes the +HTTP response status + headers. + +**ResolvedMutation** +The Go struct (`policy.ResolvedMutation`) an extProc evaluation produces: +an op with its CEL values already evaluated, ready for the gRPC server to +translate into Envoy protos. ## Lifecycle @@ -160,8 +189,9 @@ Common Expression Language. A spec-defined, sandbox-safe boolean expression language. https://github.com/google/cel-spec. **Activation** -The set of variables CEL sees when evaluating a program. In our case -always two top-level vars: `request` and `facts`. +The set of variables CEL sees when evaluating a program. `request` and +`facts` in request phases; `request`, `response` and `facts` in response +phases. **Program** A compiled CEL expression ready to be evaluated. Cached by source diff --git a/.agents/OPERATIONS.md b/.agents/OPERATIONS.md index 85f52ee..c1c19ab 100644 --- a/.agents/OPERATIONS.md +++ b/.agents/OPERATIONS.md @@ -45,7 +45,7 @@ controllers: spec: { httpGet: { path: /readyz, port: 8080 } }, } service: - main: { controller: main, ports: { http: { port: 8080 } } } + main: { controller: main, ports: { http: { port: 8080 }, grpc: { port: 9090 } } } persistence: policy: type: configMap @@ -57,10 +57,18 @@ Two pods minimum. Roll one at a time; both the readiness probe and the fail-closed boot semantics protect you from going live with a broken policy. +The extAuthz engine listens on `:8080` (HTTP), the extProc engine on +`:9090` (gRPC, flag `--grpc-port`). Expose both ports on the Service if +you use both engines; the readiness/liveness probes stay on the HTTP +port. + ## Wiring into Istio -`request-validator` is an Envoy ext-authz HTTP service. Two pieces of -Istio config are needed: +`request-validator` exposes two Envoy filters. Wire the one(s) you use. + +### extAuthz (HTTP) + +Two pieces of Istio config are needed: 1. **`extensionProvider`** in `MeshConfig` (one-off, mesh-wide): @@ -118,6 +126,16 @@ Istio config are needed: Only matched traffic hits the validator. The rest stays on your existing Istio policies. +### extProc (gRPC) + +The extProc engine is reached through an `envoyExtProc` extension +provider pointing at the gRPC port (`:9090`), selected on the target +traffic. The exact resource depends on your Istio version; point it at +the `grpc` Service port and consult the Istio docs for the current +ext_proc provider schema. A group only runs when its `parameters.phase` +matches the ext_proc message Envoy sends, so enable the body phases in +the filter config only if a policy needs the body. + ## Observability ### Logs @@ -248,7 +266,7 @@ The previous policy stays active and the failure is logged at `ERROR`. - Memory: a few tens of MB resting; peaks during reload while the previous and new `Config` coexist. 128 MiB requests are comfortable for the example policy. -- CPU: dominated by CEL evaluation. Around 0.1–0.5 ms p99 per request +- CPU: dominated by CEL evaluation. Around 0.1-0.5 ms p99 per request for the example. Scale horizontally. - File descriptors: one per URL fact fetcher keepalive + the HTTP listener. Default limits are fine. diff --git a/.agents/POLICY_DSL.md b/.agents/POLICY_DSL.md index 9f7931b..7e4a550 100644 --- a/.agents/POLICY_DSL.md +++ b/.agents/POLICY_DSL.md @@ -1,53 +1,77 @@ # POLICY_DSL.md -Reference for the YAML policy file. The user-facing version lives in -the project README; this one is denser, no preamble, and is the source -of truth for what the parser accepts. +Reference for the YAML policy file. The user-facing version lives in the +project README; this one is denser, no preamble, and is the source of +truth for what the parser accepts. + +The service runs two engines from one policy file: + +- **extAuthz**: HTTP ext-authz. A binary gatekeeper. Reads a request, + produces allow/deny. +- **extProc**: gRPC ext_proc. Inspects and mutates live traffic (request + or response headers/body) and can short-circuit with an immediate + response. + +Each group declares which engine it programs. See DECISIONS.md D-015..D-022. ## Top-level structure ```yaml -defaults: # engine-wide knobs -logging: # access-log shape and redaction -facts: # named external values referenced as facts. -groups: # ordered list of rule buckets +defaults: # engine-wide knobs, split per engine +logging: # access-log shape and redaction +facts: # named external values referenced as facts. +groups: # ordered list of rule buckets, each bound to one engine ``` -All sections are optional **except** `groups`, which must contain at -least one entry with at least one rule. +All sections are optional except `groups`, which must contain at least +one entry with at least one rule. ## `defaults` +Split per engine. Each block is self-contained. + ```yaml defaults: - action: deny # allow | deny default: deny - denyStatus: 403 # int default: 403 - denyBody: "Forbidden" # string default: "Forbidden" - maxBodyBytes: 1MiB # BytesSize default: 1MiB - allowOnError: false # bool default: false - dryRun: false # bool default: false + extAuthz: + action: deny # allow | deny default: deny + denyStatus: 403 # int default: 403 + denyBody: "Forbidden" # string default: "Forbidden" + allowOnError: false # bool default: false + maxBodyBytes: 1MiB # BytesSize default: 1MiB + extProc: + maxBodyBytes: 1MiB # BytesSize default: 1MiB + onBodyOverflow: fail # skip | fail default: fail + dryRun: false # bool, global default: false ``` -`BytesSize` accepts plain integers (bytes) and human strings: -`1024`, `1KiB`, `2MiB`, `1GiB`, `1KB`, `1MB`, `1024b`. +`BytesSize` accepts plain integers (bytes) and human strings: `1024`, +`1KiB`, `2MiB`, `1GiB`, `1KB`, `1MB`, `1024b`. + +`extAuthz.allowOnError` flips the fail-closed behaviour: when a CEL +evaluation hits an error mid-request, the verdict becomes `allow` instead +of `deny`. Used rarely, with care. + +`extAuthz` body overflow (request body larger than `maxBodyBytes`) is a +fixed fail-closed deny; it is not configurable. A gatekeeper that cannot +read the whole request does not admit it. -`allowOnError` flips the default fail-closed behaviour: when a CEL -evaluation hits an error mid-request, the verdict becomes `allow` -instead of `deny`. Used rarely, with care. +`extProc.onBodyOverflow` controls a body larger than `maxBodyBytes` in a +body phase: `skip` lets the original body pass unmutated; `fail` returns +an immediate fail-closed response. There is no `truncate`. -`dryRun` enables global shadow mode. When true, the service evaluates every -rule as normal but never enforces a deny; requests always return HTTP 200. -The access log and metrics record the verdict the policy produced. A failure -reading the request body normally fail-closes; under global `dryRun` it also -passes through with HTTP 200. +`dryRun` is global and applies to both engines. When true the service +evaluates everything but enforces nothing: extAuthz denies pass with HTTP +200; extProc mutations are computed and logged but the stream responds +CONTINUE with no mutation. Overflow paths that would deny/fail also pass. +The access log and metrics record what the policy would have done. ## `logging` ```yaml logging: - level: info # debug|info|warn|error default: info - format: json # json|console default: json - logBody: false # bool default: false + level: info # debug|info|warn|error default: info + format: json # json|console default: json + logBody: false # bool default: false redactReveal: 6 # int default: 6 excludeHeaders: [cookie, set-cookie] redactHeaders: [authorization, proxy-authorization, x-api-key, x-auth-token] @@ -57,14 +81,14 @@ logging: Behaviour: - Header keys are normalised to lowercase before exclude/redact checks. -- Redaction: if a value's length is `>= 2 * redactReveal`, the first - `redactReveal` chars are kept and the rest replaced with `*`. +- Redaction: if a value's length is at least `2 * redactReveal`, the + first `redactReveal` chars are kept and the rest replaced with `*`. Otherwise the value is fully masked. With `redactReveal: 0` every redacted value is fully masked. -- `logBody: true` adds `request.body.raw` to the access log. Off by - default because DCR payloads carry secrets. -- `--log-level` / `--log-format` CLI flags override the YAML values - when set. +- `logBody: true` adds the body to the access log. Off by default because + DCR payloads carry secrets. +- `--log-level` / `--log-format` CLI flags override the YAML values when + set. ## `facts` @@ -72,32 +96,34 @@ A list of named external values. CEL sees them as `facts.`. ```yaml facts: - - name: # required, unique across facts + - name: # required, unique across facts method: value | file | url value: # required when method=value - file: # required when method=file + file: # required when method=file path: /abs/or/rel/path - url: # required when method=url + url: # required when method=url address: https://... - interval: 10m # optional, default 10m - timeout: 15s # optional, default 15s - headers: # optional + interval: 10m # optional, default 10m + timeout: 15s # optional, default 15s + headers: # optional Authorization: "Bearer $TOKEN" ``` -Method semantics: - | Method | Loaded | Type seen by CEL | | ------ | ---------------------------------------------------- | ---------------- | | value | At YAML parse; the value is used verbatim | as declared | -| file | At `Start()`, the file is read once into a string | `string` | -| url | At `Start()` (initial) + every `interval` afterwards | `string` | +| file | At Start(), the file is read once into a string | string | +| url | At Start() (initial) plus every interval afterwards | string | URL fact failure modes: -- Initial fetch fails → policy load is rejected. The previous policy - (if any) keeps running. -- Refresh fails → previous value retained, `WARN` log emitted. +- Initial fetch fails: policy load is rejected. The previous policy (if + any) keeps running. +- Refresh fails: previous value retained, WARN log emitted. + +The interstitial HTML for the redirect-rewrite case is a `file` or `url` +fact, injected into a response via a `setBody` mutation. It never lives +in code. CEL helpers for parsing the raw string into structured data: @@ -111,89 +137,179 @@ expression stays safe before the first successful fetch. ## `groups` -A list of named buckets. Evaluated in declared order; the first group -that produces a verdict wins. If none decide, `defaults.action` applies. +A list of named buckets. Evaluated in declared order. Each group is bound +to one engine via `parameters.engine`. extAuthz groups are served by the +HTTP listener; extProc groups by the gRPC listener. ```yaml groups: - - name: # required, unique across groups - description: ... # optional - mode: firstMatch | all # optional, default firstMatch - action: allow | deny # optional, default allow; inherited by rules - match: | # optional CEL expression (default: true) - - rules: # required, non-empty - - + - name: # required, unique across groups + description: ... # optional + parameters: + engine: extAuthz | extProc # required, no default + mode: # required + phase: # required iff engine=extProc, forbidden otherwise + match: | # optional CEL bool, group guard + + rules: # required, non-empty - ``` -Group `match` is evaluated **once** per request. If false, the whole -group is skipped silently. +Group `match` is evaluated once per request. If false, the whole group is +skipped silently. It factors out a common scope (e.g. `response.status == 302`). + +### `parameters.mode` + +Per engine. The loader rejects a mode not valid for the group's engine. + +- extAuthz: `firstMatch | matchAll` + - `firstMatch`: the first rule whose `match` is true decides. If none + match, the group produces no verdict and evaluation continues. + - `matchAll`: every rule must match. The first failure produces a deny. + Defence-in-depth. +- extProc: `firstMatch | applyAll` + - `firstMatch`: the first rule whose `match` is true applies its + mutations and stops. An empty `mutations: []` is a valid "matched, + change nothing" that still stops the group. + - `applyAll`: every matching rule applies its mutations, in declaration + order. When two matching rules write the same header, the last one + wins. -### Modes +### `parameters.phase` (extProc only, required) -- **`firstMatch`** (default): the first rule whose `match` evaluates - true decides. Use for "this request is allowed if ANY of these rules - matches". -- **`all`**: every rule must match. The first failure produces the - opposite of the group's `action`. Use for defence-in-depth where every - predicate must hold. +`requestHeaders | requestBody | responseHeaders | responseBody` + +Determines which Envoy ext_proc hook the group runs on and which CEL +variables are live: + +| phase | live CEL variables | +| --------------- | --------------------------- | +| requestHeaders | request, facts | +| requestBody | request, facts | +| responseHeaders | request, response, facts | +| responseBody | request, response, facts | + +A rule that references `response` in a request phase fails to compile. ### Rule ```yaml -- name: # required, unique within the group - description: ... # optional - action: allow | deny # optional; inherited from group when omitted - match: | # CEL bool expression (default: true) +- name: # required, unique within the group + description: ... # optional + match: | # required CEL bool expression - fallthrough: next|allow|deny# default: next (only used in firstMatch groups) - dryRun: false # bool, default false + # extAuthz rules carry exactly this: + validation: + action: allow | deny + # extProc rules carry exactly this instead: + mutations: + - { op: setHeader, name: , value: } + - { op: appendHeader, name: , value: } + - { op: removeHeader, name: } + - { op: setBody, value: } + - { op: setStatus, code: } + - { op: directResponse, status: , headers: >, body: } + dryRun: false # bool, default false ``` -- A rule's effective `action` is its own when set, else the group's, - else `allow`. -- `fallthrough` controls what happens in a `firstMatch` group when this - rule's `match` is false: `next` tries the next rule; `allow`/`deny` - short-circuits with that verdict. -- `dryRun: true` enables shadow mode for the rule. The evaluator produces the - verdict and the access log and metrics record it with `dry_run: true`, - while enforcement is suppressed. +- `match` is mandatory. There is no implicit `true`. For a fallback rule + write `match: 'true'` explicitly. +- An extAuthz rule has `validation` (a singular object), never + `mutations`. An extProc rule has `mutations` (a list), never + `validation`. The loader rejects the wrong one for the engine. +- There is no `action` inheritance from the group and no `fallthrough`. +- `dryRun: true` shadows the rule: the verdict/mutations are computed, + logged and metered with `dry_run: true`, enforcement suppressed. + +### Mutation ops + +Two kinds. The incremental ops mutate the upstream response in place. +`directResponse` is different: it discards the upstream response and +serves one the rule builds (Envoy ImmediateResponse). A rule carrying +`directResponse` must not carry any incremental op; the loader rejects the +mix. In `applyAll` the first rule that fires a `directResponse` wins and +stops the group. + +| op | fields | legal phases | semantics | +| -------------- | ----------- | ----------------------------- | ------------------------------------------- | +| `setHeader` | name, value | all | upsert: overwrite if present, else create | +| `appendHeader` | name, value | all | add a value, keep existing ones | +| `removeHeader` | name | all | delete the header; no-op if absent | +| `setBody` | value | requestBody, responseBody | replace the body; recomputes Content-Length | +| `setStatus` | code | responseHeaders, responseBody | set the response status code | +| `directResponse` | status, headers, body | responseHeaders, responseBody | discard the upstream response, serve this one | + +`value` is a CEL expression returning a string; `code` a CEL expression +returning an int. These are checked for output type at load time, unlike +`match` which is checked as bool. `setBody` recomputes Content-Length; +Content-Type stays the user's responsibility (set it with `setHeader`). + +`directResponse` builds a full response from scratch and short-circuits +(maps to Envoy ImmediateResponse). Its fields: + +- `status`: int literal (e.g. 200). +- `headers`: a CEL expression returning `map`, checked for + output type at load. A map literal sets fixed headers + (`{"content-type": "text/html"}`); `response.header` carries over every + original header ("give me all of them"); filter/merge expressions pick a + subset or override. There is no inherit-by-default. Single-string + values only (no multi-value output in v1). +- `body`: a CEL expression returning a string. + +It works in `responseHeaders` even when upstream sent no body (the +bodiless `302` case). A rule with `directResponse` cannot carry any +incremental op. Under dry-run it is logged ("would directRespond") but not +served. + +### Decision flow (extAuthz, pseudo-code) + +``` +for each extAuthz group in order: + if group.match is false: continue + switch group.mode: + case firstMatch: + for each rule in order: + if rule.match is true: return rule.validation.action + continue # no rule matched, try next group + case matchAll: + for each rule in order: + if rule.match is false: return deny + return allow # whichever action makes the group hold +return defaults.extAuthz.action +``` -### Decision flow (pseudo-code) +### Mutation flow (extProc, pseudo-code) ``` -for each group in order: +for each extProc group bound to the current phase, in order: if group.match is false: continue switch group.mode: case firstMatch: for each rule in order: if rule.match is true: - return decision(rule.action) - else: apply rule.fallthrough - case all: + apply rule.mutations (may be empty); stop + case applyAll: for each rule in order: - if rule.match is false: - return decision(NOT group.action) - return decision(group.action) -return decision(defaults.action) + if rule.match is true: accumulate rule.mutations + apply accumulated mutations (last write wins per header) +respond to Envoy with the accumulated mutations, or CONTINUE if none ``` ## CEL environment -Two variables visible to every expression: +Variables visible to an expression depend on the phase (see the table +above). `request` and `response` are shaped the same way: -- **`request`** - built per request. See README "The `request` object" - for the full table. -- **`facts`** - the snapshot of the facts registry. `facts.` is - whatever the source returned (a CEL list/map for `value`, a string - for `file`/`url`). +- `method`, `scheme`, `host`, `path`, `remoteIp` (request only), + `headers` (map of lists), `header` (map of first values), `queries`, + `query`, `body` (`raw`, `size`, `contentType`, `json`/`jsonOk`, + `yaml`/`yamlOk`). +- `response` additionally exposes `status` (int). -Enabled CEL extensions: `ext.Strings()`, `ext.Encoders()`, -`ext.Lists()`, `ext.Sets()`, `ext.Math()`, `ext.Bindings()`. +Enabled CEL extensions: `ext.Strings()`, `ext.Encoders()`, `ext.Lists()`, +`ext.Sets()`, `ext.Math()`, `ext.Bindings()`. -Project-specific functions are documented in the README. Quick -reference: +Project-specific functions (full docs in the README): | Family | Functions | | -------- | --------------------------------------------------------------- | @@ -209,27 +325,28 @@ reference: ```json { "time": "...", - "level": "INFO" or "WARN", - "msg": "request decided", - "decision": "allow" or "deny", - "rule": "/" or "", - "reason": "matched" or "no group matched" or "...", + "level": "INFO or WARN", + "msg": "request decided or response processed", + "engine": "extAuthz or extProc", + "decision": "allow or deny", + "mutations": ["setHeader location", "..."], + "rule": "/ or ", + "reason": "matched or no group matched or ...", "dry_run": false, "duration_ms": 0.31, "request": { ... } } ``` -- `decision` always reflects the verdict the policy produced, even when shadow - mode suppresses enforcement. -- `dry_run: true` means the verdict was evaluated but not enforced, set by - either the matching rule's `dryRun` or the global `defaults.dryRun`. -- Log level is `WARN` for any `deny` decision, including one suppressed by - `dry_run`, and `INFO` otherwise. -- `rule == ""` means no group produced a verdict and the - `defaults.action` fired. +- extAuthz records `decision`; extProc records the `mutations` it applied + (or would apply under dry-run). +- `decision`/`mutations` always reflect what the policy produced, even + when shadow mode suppresses enforcement. +- `dry_run: true` means evaluated but not enforced. +- Log level is WARN for any `deny`, including a shadowed one, INFO + otherwise. -## Response headers emitted to Envoy +## Response headers emitted to Envoy (extAuthz) | Header | Value | | -------------- | ------------------------------------------------ | @@ -238,22 +355,35 @@ reference: | `x-rv-reason` | short, human-readable | | `x-rv-dry-run` | `true` if the rule was in shadow mode | -These are mostly for debugging; Envoy itself only acts on the HTTP -status code (200 = allow, anything else = deny). +These are for debugging; Envoy acts on the HTTP status code (200 = allow, +anything else = deny). ## Errors that stop policy load - YAML doesn't parse. -- `defaults.action` not in {allow, deny}. +- A group missing `parameters.engine`, or `engine` not in {extAuthz, extProc}. +- `mode` missing or not valid for the engine (extAuthz: firstMatch|matchAll; + extProc: firstMatch|applyAll). +- `phase` present on an extAuthz group, or missing/invalid on an extProc group. +- An extAuthz rule carrying `mutations`, or an extProc rule carrying `validation`. +- A rule missing `match`. +- `validation.action` not in {allow, deny}. +- A mutation with an unknown `op`, missing operands, or an op used in an + illegal phase (`setStatus` outside response phases, `setBody` outside + body phases, `directResponse` outside response phases). +- A rule mixing `directResponse` with any incremental op. +- A `match` expression that does not compile to bool, a mutation + `value`/`code` expression that does not compile to string/int, a + `directResponse.headers` expression that does not compile to + `map`, or any expression referencing a variable not live + in its phase. +- `defaults.extAuthz.action` not in {allow, deny}. +- `defaults.extProc.onBodyOverflow` not in {skip, fail}. - Duplicate group name, or duplicate rule name within a group. -- Empty group (no `rules`). -- `mode` not in {firstMatch, all}. -- `action` not in {allow, deny}. -- `fallthrough` not in {next, allow, deny}. -- CEL compilation error on any `match` expression. +- Empty group (no rules). - Fact spec missing required fields, duplicate name, unknown method. - Initial fetch failure for a `url` method fact. Any of these makes `policy.LoadBytes` return an error. The caller -(`cmd/main.go`) keeps the previous policy on a reload, or fails to -start on initial boot. +(`cmd/main.go`) keeps the previous policy on a reload, or fails to start +on initial boot. diff --git a/.agents/TESTING.md b/.agents/TESTING.md index b9e65ed..864546e 100644 --- a/.agents/TESTING.md +++ b/.agents/TESTING.md @@ -52,11 +52,12 @@ becomes invisible. | Package | What's covered | | ------------- | ------------------------------------------------------------------------------------------------------------------------------- | -| `celenv` | Smoke test of every custom function (inCIDR, glob, parseJSON, parseURL, sha256Hex, jsonPath, now, has, firstOr, isPrivateIP...) | +| `celenv` | Smoke test of every custom function (inCIDR, glob, parseJSON, parseURL, sha256Hex, jsonPath, now, has, firstOr, isPrivateIP...), per-scope compilation (response var rejected in request scope), typed compile/eval (bool/string/int/stringmap) | | `configwatch` | In-place write, save-via-rename, kubelet `..data` swap, debounce of bursts | | `facts` | inline values, file source, URL fetch + refresh, URL fetch failure keeps previous value, validation (dupes, missing fields) | -| `httpserver` | Allow/deny end-to-end, DCR body validation, hot-reload swap, access log (exclude/redact headers, query redact, console + json) | -| `policy` | Evaluator (firstMatch / all / dryRun / fallthrough / action inheritance), example/policy.yaml flow, URL fact integration | +| `httpserver` | extAuthz allow/deny end-to-end, DCR body validation, body overflow deny, hot-reload swap, access log (exclude/redact headers, query redact, console + json) | +| `policy` | extAuthz evaluator (firstMatch / matchAll / dryRun, no fallthrough or inheritance), extProc EvaluateProc (firstMatch / applyAll, mutations, directResponse, fail-safe), engine isolation (each engine ignores the other's groups) and per-phase filtering, load-time validation errors, scope wiring, example/policy.yaml flow, URL fact integration | +| `grpcserver` | ext_proc stream mapping per phase, header/body/status mutations to Envoy protos, directResponse short-circuit (ImmediateResponse), dry-run suppression, body overflow skip/fail, pseudo-header parsing | ## How to add a test diff --git a/README.md b/README.md index 8cbeaec..731d1f0 100644 --- a/README.md +++ b/README.md @@ -1,58 +1,111 @@ # request-validator -A small Envoy / Istio **ext-authz** service that says `allow` or `deny` -for every request, based on a YAML policy you write in -[CEL](https://github.com/google/cel-spec). +A small Envoy / Istio service implementing both HTTP **ext-authz** (admit or deny requests) and gRPC **ext_proc** (inspect and mutate live traffic), guided by a single declarative YAML policy written in [CEL](https://github.com/google/cel-spec). -It exists for the cases plain `AuthorizationPolicy` cannot reach: -inspecting the request body, mixing CIDRs and JWT claims with JSON -contents, validating OAuth `redirect_uris`, blocking specific paths -during certain hours, and other things that don't fit the -"method + path + header" model. +It exists for cases that plain `AuthorizationPolicy` cannot reach. On the deciding side: inspecting the request body, mixing CIDRs and JWT claims with JSON contents, validating OAuth `redirect_uris`, or blocking specific paths during certain hours. On the rewriting side: turning an untrusted redirect into a warning page, stripping internal headers off a response, injecting a header before it reaches the upstream, or replacing a body or status code in transit. -## Two-minute tour +## Table of contents -The shortest meaningful policy: a single rule that allows POST requests -to a Keycloak Dynamic Client Registration endpoint when the client IP -sits in a small private range. +- [The two engines](#the-two-engines) +- [Engine 1: extAuthz (deciding)](#engine-1-extauthz-deciding) + - [A two-minute tour](#a-two-minute-tour) + - [Recipes](#recipes) + - [How extAuthz picks a verdict](#how-extauthz-picks-a-verdict) + - [extAuthz defaults](#extauthz-defaults) +- [Engine 2: extProc (mutating)](#engine-2-extproc-mutating) + - [Phases and modes](#phases-and-modes) + - [Mutations](#mutations) + - [The directResponse operation](#the-directresponse-operation) + - [extProc defaults](#extproc-defaults) + - [Small recipes](#small-recipes) + - [Recipe: rewrite an untrusted redirect into a warning page](#recipe-rewrite-an-untrusted-redirect-into-a-warning-page) +- [Common to both engines](#common-to-both-engines) + - [Global dry-run](#global-dry-run) + - [What CEL sees](#what-cel-sees) + - [Facts](#facts) + - [CEL function reference](#cel-function-reference) + - [Logging](#logging) +- [Operating the service](#operating-the-service) + - [Running it](#running-it) + - [Endpoints](#endpoints) + - [Hot reload](#hot-reload) + - [Flags](#flags) + - [Response headers](#response-headers) + - [Plugging into Istio](#plugging-into-istio) +- [License](#license) + +## The two engines + +request-validator runs two engines from one policy file. Every group picks which one it programs with `parameters.engine`. They solve different problems; use either or both. + +**extAuthz (HTTP, the gatekeeper).** Envoy asks "should this request go through?" and we answer `allow` or `deny`. Reach for it to admit or block traffic: let only known clients hit an endpoint, reject a DCR body that declares a wildcard `redirect_uri`, close a realm on a public hostname. It never changes the request or the response, it only decides. + +**extProc (gRPC, the surgeon).** Envoy streams the request and the response through us, and we can inspect and rewrite them: set or drop headers, replace a body, change a status code, or discard the upstream response and serve our own. Reach for it when deciding is not enough and you need to change what flows: rewrite an untrusted redirect into a warning page, strip leaking internal headers off a response, add a header before it reaches the upstream. + +Rule of thumb: if the answer is yes or no, it is extAuthz; if you need to touch headers, body or status, it is extProc. + +How they differ at a glance: + +| | extAuthz | extProc | +| --- | --- | --- | +| Envoy filter | HTTP ext-authz | gRPC ext_proc | +| Listens on | `--port` (8080) | `--grpc-port` (9090) | +| Produces | a verdict (`allow` / `deny`) | mutations, or a direct response | +| Acts on | the request | the request and the response | +| Rule body | `validation` | `mutations` | +| Group modes | `firstMatch`, `matchAll` | `firstMatch`, `applyAll` | + +This README documents extAuthz in full first, then extProc in full, then the parts both engines share. Read the engine you need; the common sections at the end apply to both, and call out where the two behave differently. + +## Engine 1: extAuthz (deciding) + +The extAuthz engine answers one question per request: `allow` or `deny`. Envoy forwards the request to us over HTTP ext-authz, we evaluate the policy, and Envoy enforces the verdict. This engine never changes the request; it only decides. + +### A two-minute tour + +The shortest meaningful policy: a single rule that allows POST requests to a Keycloak Dynamic Client Registration endpoint when the client IP sits in a small private range. ```yaml defaults: - action: deny + extAuthz: + action: deny groups: - name: dcr-internal - action: allow + parameters: + engine: extAuthz + mode: firstMatch rules: - name: from-internal-cidr match: | request.method == 'POST' && request.path.startsWith('/realms/mcp/clients-registrations') && inCIDR(request.remoteIp, ['10.0.0.0/8']) + validation: + action: allow ``` The pieces: -- `defaults.action` runs when nothing else decides. We default to - `deny`, so missing a case never accidentally lets a request through. -- A `group` is a small bucket of rules with a shared verdict - (`action: allow`). Rules inside a group inherit it. -- Each `rule` has a CEL boolean expression in `match`. When it - evaluates to `true` the rule fires and the verdict is applied. -- `inCIDR(...)` is one of the small set of helper functions - request-validator adds on top of CEL. +- `defaults.extAuthz.action` runs when no rule decides. We default to `deny`, so missing a case never accidentally lets a request through. +- A `group` matches requests and binds them to an engine (`parameters.engine: extAuthz`) with a decision mode (`parameters.mode: firstMatch`). +- Each `rule` has a mandatory CEL boolean expression in `match`. When it evaluates to `true` the rule fires and its `validation.action` is returned. +- `inCIDR(...)` is one of the helper functions request-validator adds on top of CEL. + +That is the shape of the language for this engine. The recipes below build on it. -That is the whole shape of the language. The rest of the README -introduces it piece by piece with realistic examples. +### Recipes -## A few realistic recipes +Every recipe here uses `engine: extAuthz`, so each one returns a verdict. -### 1. Allow an admin endpoint from an office subnet during work hours +#### 1. Allow an admin endpoint from an office subnet during work hours ```yaml groups: - name: admin-business-hours - action: allow + parameters: + engine: extAuthz + mode: firstMatch rules: - name: office-during-the-day match: | @@ -60,56 +113,61 @@ groups: inCIDR(request.remoteIp, ['203.0.113.0/24']) && now().getHours('UTC') >= 7 && now().getHours('UTC') < 19 + validation: + action: allow ``` -`now()` returns the current UTC time. CEL's timestamp accessors -(`getHours`, `getDayOfWeek`, `getMonth`...) cover the typical -schedule checks without dragging in cron. +`now()` returns the current UTC time. CEL's timestamp accessors (`getHours`, `getDayOfWeek`, `getMonth`...) cover the typical schedule checks without dragging in cron. -### 2. Require a header on a specific path +#### 2. Require a header on a specific path ```yaml groups: - name: webhook-needs-signature - action: allow + parameters: + engine: extAuthz + mode: firstMatch rules: - name: signed-with-x-hub-signature match: | request.path.startsWith('/hooks/github') && has('x-hub-signature-256', request.headers) && request.header['x-hub-signature-256'].startsWith('sha256=') + validation: + action: allow ``` -`has('x-hub-signature-256', request.headers)` is a small helper: -the header must exist and have a non-empty value. After that we look -at the first value directly via `request.header[...]`. +`has('x-hub-signature-256', request.headers)` is a helper: the header must exist and have a non-empty value. After that we look at the first value directly via `request.header[...]`. -### 3. Block a sensitive realm on public hostnames +#### 3. Block a sensitive realm on public hostnames ```yaml groups: - name: keep-master-realm-private - action: deny + parameters: + engine: extAuthz + mode: firstMatch rules: - name: no-master-on-public-hosts match: | request.host in ['auth.example-1.com', 'auth.example-2.com'] && request.path.startsWith('/realms/master') + validation: + action: deny ``` -A whole group declared as `action: deny` keeps the intent obvious at -a glance. +Explicit validation actions on each rule keep the intent clear at a glance. -### 4. Decide based on what the JSON body says +#### 4. Decide based on what the JSON body says -This is the kind of check Istio's `AuthorizationPolicy` cannot make -on its own. Here we only allow Dynamic Client Registration when every -`redirect_uri` in the body belongs to an approved provider: +This is the kind of check Istio's `AuthorizationPolicy` cannot make on its own. Here we only allow Dynamic Client Registration when every `redirect_uri` in the body belongs to an approved provider: ```yaml groups: - name: dcr-trusted-redirects - action: allow + parameters: + engine: extAuthz + mode: firstMatch match: | request.method == 'POST' && request.path.matches('^/realms/mcp/clients-registrations(/.*)?$') && @@ -119,120 +177,452 @@ groups: match: | request.body.json.redirect_uris.all(u, u.startsWith('https://antigravity.google/')) + validation: + action: allow - name: chatgpt match: | request.body.json.redirect_uris.all(u, u.matches('^https://([a-z0-9-]+\\.)?openai\\.com/.+$')) + validation: + action: allow ``` -The group's `match` plays the role of a shared filter (POST, the right -path, parseable JSON body). Each rule below adds the provider-specific -test on top of that. +The group's `match` plays the role of a shared filter (POST, the right path, parseable JSON body). Each rule below adds the provider-specific test on top of that. -### 5. Defence in depth for an admin area +#### 5. Defence in depth for an admin area -When you want several independent conditions to all hold, set the -group's `mode` to `all`: +When you want several independent conditions to all hold, set the group's `mode` to `matchAll`: ```yaml groups: - name: admin-defence-in-depth - action: allow - mode: all + parameters: + engine: extAuthz + mode: matchAll match: | request.path.startsWith('/admin') rules: - name: from-internal-network match: inCIDR(request.remoteIp, ['10.0.0.0/8', '192.168.0.0/16']) + validation: + action: allow - name: has-admin-claim match: request.header['x-user-groups'].contains('platform-admins') + validation: + action: allow - name: no-debug-header match: '!has("x-debug", request.headers)' + validation: + action: allow +``` + +In `matchAll` mode, every rule in the group must match. The first failure causes the group to immediately produce a deny verdict. + +A larger example with several groups, facts, and the real DCR flow lives in [`examples/policy.yaml`](examples/policy.yaml). Have a read once you've gone through the rest of this document. + +### How extAuthz picks a verdict + +Groups are evaluated in declared order. Only groups with `engine: extAuthz` take part here; `extProc` groups are ignored by this engine. The first extAuthz group that produces a verdict wins. If no group produces one, the fallback in `defaults.extAuthz.action` applies. + +Inside a group, the optional `match` expression filters incoming requests. If it evaluates to false, the group is silently skipped. Otherwise, the group's `parameters.mode` runs: + +- **`firstMatch`**: the first rule whose `match` evaluates to true decides. The group produces that rule's `validation.action` and evaluation stops. If no rule matches, the group produces no verdict and evaluation proceeds to the next group. +- **`matchAll`**: every rule in the group must match. The first rule that fails to match makes the group produce a `deny` verdict immediately. If all rules match, the group produces an `allow` verdict. + +A rule's `match` expression is mandatory. A fallback rule must spell out `match: 'true'`. There is no inheritance of actions and no fallthrough between rules: each rule states its own `validation.action`. + +### extAuthz defaults + +The extAuthz engine reads its defaults from `defaults.extAuthz`: + +```yaml +defaults: + extAuthz: + action: deny # Fallback verdict when no group decides (allow | deny) + denyStatus: 403 # HTTP status returned on deny + denyBody: "Forbidden" # HTTP body returned on deny + allowOnError: false # If true, a CEL evaluation error allows instead of denies + maxBodyBytes: 1MiB # Max request body buffered for inspection +``` + +`BytesSize` values accept plain integers (bytes) or human strings: `1024`, `1KiB`, `2MiB`, `1GiB`, `1KB`, `1MB`, `1024b`. + +Two behaviours worth knowing: + +- **`allowOnError`**: by default any evaluation error (a missing key, a type error) fails closed to `deny`. Set this to `true` to fail open to `allow` instead. Use it sparingly. +- **Body overflow is a fixed fail-closed deny**: if the request body is larger than `maxBodyBytes`, the engine denies, full stop. This is not configurable: a gatekeeper that cannot read the whole request must not admit it. + +There is also a global `defaults.dryRun` that affects both engines, covered in [Global dry-run](#global-dry-run). + +## Engine 2: extProc (mutating) + +The extProc engine is the other half. Instead of deciding, it inspects and rewrites traffic. It runs as a gRPC Envoy `ext_proc` server: Envoy streams the request and the response through it, and the policy can set or drop headers, replace a body, change a status code, or discard the upstream response and serve its own. + +A minimal extProc group, which stamps a header onto every response: + +```yaml +groups: + - name: stamp-a-header + parameters: + engine: extProc + phase: responseHeaders + mode: applyAll + rules: + - name: add-marker + match: "true" + mutations: + - op: setHeader + name: x-processed-by + value: "'request-validator'" ``` -In `all` mode, one rule failing causes the group to deny. It is the -"every box on this checklist must be ticked" idiom. - -A larger example with several groups, facts and the real DCR -flow lives in [`examples/policy.yaml`](examples/policy.yaml). Have a -read once you've gone through the rest of this document. - -## How the engine picks a verdict - -Groups are evaluated in order. Each group decides on its own and the -**first** group that produces a verdict wins; if none do, -`defaults.action` takes over. - -Inside a group, the optional `match` at the group level is a -"do I apply to this request at all?" filter. If it returns false the -group is skipped silently. Otherwise the group's `mode` runs: - -- **`firstMatch`** (the default): the first rule whose `match` - evaluates to true decides. Use it for "any of these rules is enough". -- **`all`**: every rule must return true. The first failing rule - causes the group to deny. - -A rule's effective `action` is its own when declared, otherwise the -group's, otherwise `allow`. This lets a whole group of allow rules -keep one outlier that flips to `deny` for an anomaly check. - -For `firstMatch` groups, a rule whose `match` returned false moves on -to the next rule by default (`fallthrough: next`). Setting -`fallthrough` to `allow` or `deny` short-circuits the group with that -verdict. - -A rule can also be marked `dryRun: true`: it is evaluated and the verdict -is recorded, but a `deny` is not enforced and the request goes through. The -access log shows the verdict it produced (`decision: deny`) with -`dry_run: true`, so you see what a tightening would block before flipping it -on. Setting `defaults.dryRun: true` does the same for the whole policy at -once: every request is evaluated, nothing is enforced, and the log and -metrics report the verdict each request would have received. - -## What CEL sees about a request - -Every CEL expression has access to two top-level variables: `request` -(the incoming HTTP request, populated for each call) and `facts` -(see the next section). - -| Field | Type | Notes | -| -------------------------- | --------------------------- | ------------------------------------------------------ | -| `request.method` | `string` | HTTP verb | -| `request.scheme` | `string` | http / https (from X-Forwarded-Proto) | -| `request.host` | `string` | request authority, no port | -| `request.path` | `string` | URL path | -| `request.remoteIp` | `string` | client IP (X-Forwarded-For first hop, else RemoteAddr) | -| `request.headers` | `map>` | all headers, keys lowercased | -| `request.header` | `map` | first value per header (lowercased keys) | -| `request.queries` | `map>` | all query parameters | -| `request.query` | `map` | first value per query parameter | -| `request.body.raw` | `string` | full body, capped at `defaults.maxBodyBytes` | -| `request.body.size` | `int` | bytes | -| `request.body.contentType` | `string` | shortcut for `request.header['content-type']` | -| `request.body.json` | `dyn` | parsed JSON, or `{}` when not JSON | -| `request.body.jsonOk` | `bool` | body parsed successfully as JSON | -| `request.body.yaml` | `dyn` | parsed YAML, or `{}` when not YAML | -| `request.body.yamlOk` | `bool` | body parsed successfully as YAML | - -For the body to arrive at all, Envoy needs to be told to forward it -(see "Plugging into Istio" below). - -## Facts: data that changes more often than the policy - -Sometimes the data your policy depends on changes too often to keep -inline. The set of CIDRs OpenAI publishes for ChatGPT actions is the -canonical example: it can change every few weeks. Hardcoding the list -into the policy file means a redeploy each time. - -`facts` solves this. You declare a named value at the top of the file -and reference it from CEL as `facts.`. Three ways to load one: - -| Method | When | What CEL sees | -| ------- | ---------------------------------------------------- | ---------------- | -| `value` | Inline in YAML, parsed at load time | The value as declared | -| `file` | Read from disk at startup or reload | A string with the file contents | -| `url` | Fetched periodically by a background goroutine | A string with the last successful body | +Two things are new compared to extAuthz: a `phase` (extProc acts at a specific point in the request/response lifecycle) and a `mutations` block instead of `validation` (the rule changes traffic instead of returning a verdict). Note also that header values are CEL expressions: `"'request-validator'"` is a CEL string literal, quotes included. + +### Phases and modes + +An extProc group must declare a `phase` in addition to `engine` and `mode`. + +**`parameters.phase`** is the Envoy processing stage the group runs at. It also fixes which CEL variables are live and which mutation ops are legal: + +| Phase | Runs on | Live CEL variables | +| ----------------- | ------------------ | ------------------------------ | +| `requestHeaders` | request headers | `request`, `facts` | +| `requestBody` | request body | `request`, `facts` | +| `responseHeaders` | response headers | `request`, `response`, `facts` | +| `responseBody` | response body | `request`, `response`, `facts` | + +The `response` variable only exists in the response phases. An expression that references `response` in a request phase fails to compile at policy load. + +**`parameters.mode`** for extProc: + +- **`firstMatch`**: applies the mutations of the first rule whose `match` is true, then stops the group. An empty list `mutations: []` is valid: it counts as a match that changes nothing but still halts the group. +- **`applyAll`**: every rule that matches contributes its mutations, in declaration order. If two rules write the same header, the last write wins. + +(The extAuthz modes are `firstMatch` and `matchAll`; extProc's second mode is `applyAll`. The names differ because the behaviour differs: `matchAll` is a verdict conjunction, `applyAll` is a mutation accumulation.) + +### Mutations + +Rules in an extProc group carry a `mutations` list instead of a `validation`: + +```yaml +rules: + - name: modify-headers + match: "true" + mutations: + - op: setHeader + name: x-custom-header + value: "request.host + '/custom'" +``` + +| Operation | Fields | Legal phases | Semantics | +| ---------------- | --------------------------- | --------------------------------- | ------------------------------------------------------------------------ | +| `setHeader` | `name`, `value` | all | Overwrites the header if present, creates it if absent. | +| `appendHeader` | `name`, `value` | all | Adds the value as an extra header entry, keeping existing ones. | +| `removeHeader` | `name` | all | Deletes the header. Idempotent (no-op if absent). | +| `setBody` | `value` | `requestBody`, `responseBody` | Replaces the whole payload and recomputes Content-Length. | +| `setStatus` | `code` | `responseHeaders`, `responseBody` | Overrides the HTTP response status code. | +| `directResponse` | `status`, `headers`, `body` | `responseHeaders`, `responseBody` | Discards the upstream response and serves a custom one. See below. | + +`value` is a CEL expression returning a string; `code` is a CEL expression returning an integer. Both are type-checked at policy load. `setBody` recomputes Content-Length, but not Content-Type: if the payload format changes, set `content-type` yourself with `setHeader`. + +### The directResponse operation + +`directResponse` does not mutate the upstream response; it throws it away and serves a brand-new one (mapping to Envoy's `ImmediateResponse`). It is the right tool when you want to stop the upstream response and show your own, for example turning a redirect into a warning page. + +It is legal in `responseHeaders` and `responseBody`. It works in `responseHeaders` even when the upstream sent no body (the bodiless `302` case), which `setBody` cannot do because the body phase never fires for an empty body. + +A rule with `directResponse` is exclusive: it must be the only mutation in that rule. Mixing it with `setHeader`, `setBody`, etc. is rejected at load, because those incremental ops would mutate a response that is being discarded anyway. In `applyAll`, the first `directResponse` that fires wins and short-circuits the rest. + +Fields: + +- `status`: an integer literal (e.g. `200`). +- `headers`: a CEL expression returning `map`. Use a map literal for fixed headers (`{"content-type": "text/html"}`), or `response.header` to carry over every original header. Map merging is not available in this CEL build, so it is one or the other, not "all of the originals plus an override". +- `body`: a CEL expression returning a string. + +Under dry-run (global or per-rule) a `directResponse` is computed and logged ("would directRespond") but not served; the stream continues. + +### extProc defaults + +The extProc engine reads its defaults from `defaults.extProc`: + +```yaml +defaults: + extProc: + maxBodyBytes: 1MiB # Max request/response body buffered for a body phase + onBodyOverflow: fail # What to do when a body exceeds maxBodyBytes (skip | fail) +``` + +When a body in a body phase exceeds `maxBodyBytes`: + +- `skip`: the original body passes through unmutated (the policy does not run on it). +- `fail`: the engine returns an immediate fail-closed response. + +Unlike extAuthz, where overflow is a fixed deny, here you choose. `skip` favours not breaking large responses; `fail` favours not letting an uninspected body through. There is no `truncate` option, because acting on half a body is worse than either. + +There is also a global `defaults.dryRun` that affects both engines, covered in [Global dry-run](#global-dry-run). + +### Small recipes + +Short, single-purpose examples. Each uses `engine: extProc`. + +#### Strip internal headers off every response + +Keep server fingerprints and internal hints from leaking to clients. `applyAll` so every matching rule contributes, and `match: "true"` so it always fires. + +```yaml +groups: + - name: strip-leaky-headers + parameters: + engine: extProc + phase: responseHeaders + mode: applyAll + rules: + - name: drop-fingerprints + match: "true" + mutations: + - op: removeHeader + name: server + - op: removeHeader + name: x-powered-by +``` + +#### Add security headers to every response + +```yaml +groups: + - name: security-headers + parameters: + engine: extProc + phase: responseHeaders + mode: applyAll + rules: + - name: harden + match: "true" + mutations: + - op: setHeader + name: x-frame-options + value: "'DENY'" + - op: setHeader + name: x-content-type-options + value: "'nosniff'" + - op: setHeader + name: strict-transport-security + value: "'max-age=63072000; includeSubDomains'" +``` + +#### Inject a header before the upstream sees it + +Runs on `requestHeaders`, so only `request` and `facts` are live (no `response` yet). Useful to pass a derived value to the backend. + +```yaml +groups: + - name: tag-upstream-request + parameters: + engine: extProc + phase: requestHeaders + mode: firstMatch + rules: + - name: forward-client-network + match: inCIDR(request.remoteIp, ['10.0.0.0/8']) + mutations: + - op: setHeader + name: x-client-tier + value: "'internal'" +``` + +#### Normalise an upstream error status + +Map a leaky upstream `500` to a generic `503` without touching the body. `setStatus` only takes the integer code; pair it with a `setHeader` if you also want to mark it. + +```yaml +groups: + - name: mask-5xx + parameters: + engine: extProc + phase: responseHeaders + mode: firstMatch + rules: + - name: five-hundred-to-503 + match: "response.status == 500" + mutations: + - op: setStatus + code: "503" + - op: setHeader + name: x-rv-normalised + value: "'true'" +``` + +#### Append a value without clobbering existing ones + +`appendHeader` adds an entry and keeps the ones already there, unlike `setHeader` which overwrites. Handy for multi-valued headers. + +```yaml +groups: + - name: extra-vary + parameters: + engine: extProc + phase: responseHeaders + mode: firstMatch + rules: + - name: vary-on-language + match: "true" + mutations: + - op: appendHeader + name: vary + value: "'Accept-Language'" +``` + +#### Match on the response body, rewrite a header + +Body phases (`responseBody` here) need Envoy to buffer the body (see [Plugging into Istio](#plugging-into-istio)). Once buffered, `response.body.json` is available. + +```yaml +groups: + - name: flag-error-payloads + parameters: + engine: extProc + phase: responseBody + mode: firstMatch + rules: + - name: tag-json-errors + match: | + response.body.jsonOk && + has('error', response.body.json) + mutations: + - op: setHeader + name: x-rv-upstream-error + value: "response.body.json.error" +``` + +### Recipe: rewrite an untrusted redirect into a warning page + +The flagship extProc use case. During Dynamic Client Registration, Keycloak may answer with a `302` redirect whose `Location` points to an untrusted domain. Instead of letting the browser follow it, we discard the redirect and serve a warning page (HTTP `200`) that asks the user to confirm before leaving. The user reaches the original destination only if they click through. + +Why `200` and not the original `302`: while the response is a redirect, the browser follows it and never renders a body. Turning it into a `200` with HTML is what makes the browser show the page; the real redirect becomes a deliberate click. + +Why base64: the original `Location` is supplied by an untrusted DCR client, so it is attacker input. Embedding it raw in the HTML would open an XSS hole. We base64-encode it in the CEL expression (`base64.encode`) and the page's button decodes it client-side (`atob`). + +```yaml +facts: + - name: trustedRedirectDomains + method: value + value: + - example-1.com + - example-2.com + + - name: interstitialHtml + method: file + file: + path: /etc/policy/interstitial.html + +groups: + - name: keycloak-302-interceptor + parameters: + engine: extProc + phase: responseHeaders + mode: firstMatch + match: | + response.status == 302 && + has('location', response.headers) + rules: + - name: redirect-to-warning + match: | + !facts.trustedRedirectDomains.exists(d, + parseURL(response.header['location']).host == d || + parseURL(response.header['location']).host.endsWith('.' + d) + ) + mutations: + - op: directResponse + status: 200 + headers: '{"content-type": "text/html"}' + body: "facts.interstitialHtml.replace('__TARGET_B64__', base64.encode(response.header['location']))" +``` + +The interstitial HTML is loaded once as a `file` fact and carries a `__TARGET_B64__` placeholder that the `body` expression fills in. A minimal page lives in [`examples/interstitial.html`](examples/interstitial.html). The Istio wiring for this engine is in [`examples/config-for-istio-extproc.yaml`](examples/config-for-istio-extproc.yaml). + +## Common to both engines + +Everything below applies to both engines. Where the two behave differently, it is called out. + +### Global dry-run + +`defaults.dryRun: true` puts the whole policy in shadow mode: every rule is evaluated and logged, but nothing is enforced. It applies to both engines, with the engine-appropriate meaning: + +- **extAuthz**: a `deny` verdict is logged as if it fired, but the request is allowed through with HTTP `200`. A body overflow that would deny also passes. +- **extProc**: mutations (including `directResponse`) are computed and logged ("would mutate", "would directRespond"), but the gRPC stream returns CONTINUE without applying anything. A body overflow that would `fail` does not block the stream. + +There is also a per-rule `dryRun: true` that shadows a single rule instead of the whole policy. In both cases the logs and metrics record what the rule would have done, so you can preview a change against live traffic before enforcing it. + +```yaml +defaults: + dryRun: false # shadow mode for both engines (default false) +``` + +### What CEL sees + +Every CEL expression (a `match`, a mutation `value`/`code`, a `directResponse.headers`/`body`) reads from a small set of variables: + +- `request`: the incoming HTTP request. Always available. +- `response`: the upstream HTTP response. Available only in the response phases of the extProc engine. +- `facts`: external datasets you declare (see [Facts](#facts)). + +Which variables are live depends on context. extAuthz always sees `request` and `facts`. extProc sees them too, plus `response` in response phases: + +| Context | Live CEL variables | +| ----------------------------- | ------------------------------ | +| extAuthz (any rule) | `request`, `facts` | +| extProc `requestHeaders` | `request`, `facts` | +| extProc `requestBody` | `request`, `facts` | +| extProc `responseHeaders` | `request`, `response`, `facts` | +| extProc `responseBody` | `request`, `response`, `facts` | + +An expression that references `response` where it is not live fails to compile at policy load. + +`request` and `response` share a similar schema. `request` has the request-specific fields (method, host, path, query, remoteIp); `response` adds `status`. Both have `headers`/`header`/`body`. + +| Field | Type | Context | Notes | +| --------------------------- | --------------------------- | -------- | ------------------------------------------------------------- | +| `request.method` | `string` | Request | HTTP verb | +| `request.scheme` | `string` | Request | http or https (from X-Forwarded-Proto) | +| `request.host` | `string` | Request | Request authority, no port | +| `request.path` | `string` | Request | URL path | +| `request.remoteIp` | `string` | Request | Client IP (X-Forwarded-For first hop, else RemoteAddr) | +| `request.headers` | `map>` | Request | All headers, keys lowercased | +| `request.header` | `map` | Request | First value per header (lowercased keys) | +| `request.queries` | `map>` | Request | All query parameters | +| `request.query` | `map` | Request | First value per query parameter | +| `request.body.raw` | `string` | Request | Full body, capped at `defaults.extAuthz.maxBodyBytes` | +| `request.body.size` | `int` | Request | Bytes | +| `request.body.contentType` | `string` | Request | Shortcut for `request.header['content-type']` | +| `request.body.json` | `dyn` | Request | Parsed JSON, or `{}` when not JSON | +| `request.body.jsonOk` | `bool` | Request | Body parsed successfully as JSON | +| `request.body.yaml` | `dyn` | Request | Parsed YAML, or `{}` when not YAML | +| `request.body.yamlOk` | `bool` | Request | Body parsed successfully as YAML | +| `response.status` | `int` | Response | Upstream HTTP status code | +| `response.headers` | `map>` | Response | All response headers, keys lowercased | +| `response.header` | `map` | Response | First value per response header (lowercased keys) | +| `response.body.raw` | `string` | Response | Full response body, capped at `defaults.extProc.maxBodyBytes` | +| `response.body.size` | `int` | Response | Bytes | +| `response.body.contentType` | `string` | Response | Shortcut for `response.header['content-type']` | +| `response.body.json` | `dyn` | Response | Parsed JSON, or `{}` when not JSON | +| `response.body.jsonOk` | `bool` | Response | Body parsed successfully as JSON | +| `response.body.yaml` | `dyn` | Response | Parsed YAML, or `{}` when not YAML | +| `response.body.yamlOk` | `bool` | Response | Body parsed successfully as YAML | + +For a body (request or response) to be present at all, Envoy must be configured to buffer and forward it. See [Plugging into Istio](#plugging-into-istio). + +### Facts + +Sometimes the data your policy depends on changes too often to keep inline. The set of CIDRs OpenAI publishes for ChatGPT actions is the canonical example: it can change every few weeks. Hardcoding the list into the policy file means a redeploy each time. + +`facts` solves this. You declare a named value at the top of the file and reference it from CEL as `facts.`. Facts are shared by both engines. Three ways to load one: + +| Method | When | What CEL sees | +| ------- | ---------------------------------------------- | -------------------------------------- | +| `value` | Inline in YAML, parsed at load time | The value as declared | +| `file` | Read from disk at startup or reload | A string with the file contents | +| `url` | Fetched periodically by a background goroutine | A string with the last successful body | A small example with all three: @@ -255,7 +645,7 @@ facts: address: https://openai.com/chatgpt-actions.json interval: 10m timeout: 15s - headers: # optional, for private feeds + headers: # optional, for private feeds Authorization: "Bearer $TOKEN" ``` @@ -265,17 +655,14 @@ Using them from CEL is just dotted access: inCIDR(request.remoteIp, facts.internalCidrs) ``` -`file` and `url` facts arrive as raw strings, so you parse them -on the spot: +`file` and `url` facts arrive as raw strings, so you parse them on the spot: ```cel inCIDR(request.remoteIp, parseJSON(facts.chatgptFeed).prefixes.map(p, p.ipv4Prefix)) ``` -`parseJSON` and `parseYAML` return an empty map `{}` if the input is -empty, null or malformed. That keeps expressions safe before the -first fetch lands. Typical pattern is to guard the group's `match`: +`parseJSON` and `parseYAML` return an empty map `{}` if the input is empty, null or malformed. That keeps expressions safe before the first fetch lands. The typical pattern is to guard the group's `match`: ```yaml match: | @@ -283,44 +670,97 @@ match: | facts.chatgptFeed != null && facts.chatgptFeed != "" ``` -If the first fetch of a `url` fact fails outright, the **policy load -is rejected** and the previous policy stays active. Subsequent failed -refreshes log a warning and keep serving the last good value. This is -intentional: we'd rather hold on to stale data than open or close the -gate based on an empty feed. +If the first fetch of a `url` fact fails outright, the **policy load is rejected** and the previous policy stays active. Subsequent failed refreshes log a warning and keep serving the last good value. This is intentional: we would rather hold on to stale data than open or close the gate based on an empty feed. + +### CEL function reference + +CEL ships a small standard library; we enable `ext.Strings()`, `ext.Encoders()`, `ext.Lists()`, `ext.Sets()`, `ext.Math()` and `ext.Bindings()` on top of it. These are available to every expression in both engines. The project also registers the functions below. + +#### Network + +| Function | Signature | Description | +| -------------- | ------------------------------------------------- | ----------------------------------------------------------------------------------------------- | +| `inCIDR` | `inCIDR(ip: string, cidrs: list) -> bool` | True if `ip` belongs to any of the listed CIDRs. Plain IPs are accepted (auto-`/32` or `/128`). | +| `ipFamily` | `ipFamily(ip: string) -> string` | `"ipv4"`, `"ipv6"` or `""`. | +| `isPrivateIP` | `isPrivateIP(ip: string) -> bool` | RFC1918, RFC4193, link-local. | +| `isLoopbackIP` | `isLoopbackIP(ip: string) -> bool` | `127.0.0.0/8`, `::1`. | +| `parseURL` | `parseURL(s: string) -> map` | Returns `{scheme, host, port, path, query, fragment, username, password}`. | + +#### Strings (glob) + +| Function | Signature | Description | +| --------- | ---------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------- | +| `glob` | `glob(s: string, pattern: string) -> bool` | Shell-style globs. `*` matches anything except `/`, `**` matches everything, `?` is one char, `[abc]` is a class. | +| `globAny` | `globAny(s: string, patterns: list) -> bool` | True if any glob in the list matches. | + +For substring, replace, split, lower, upper and similar, use CEL's `ext.Strings()` directly: `s.lower()`, `s.split(',')`, `s.replace(a, b)`, etc. + +#### Encoding and hashing + +| Function | Signature | Description | +| -------------------- | ------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------ | +| `sha256Hex` | `sha256Hex(s: string) -> string` | Lowercase hex of SHA-256(s). | +| `parseJWTUnverified` | `parseJWTUnverified(token: string) -> map` | Returns `{header, payload}` parsed JSON. **Does not** verify the signature; use only when another component already did. | + +`base64.encode` and `base64.decode` come from `ext.Encoders()`. + +#### Time + +| Function | Signature | Description | +| -------- | -------------------- | ---------------------------------------------------------------------- | +| `now` | `now() -> timestamp` | Current UTC time. CEL accessors (`getHours`, `getDayOfWeek`...) apply. | + +#### Structured data + +| Function | Signature | Description | +| ----------- | ------------------------------------------------ | ------------------------------------------------------------------------------------------------------- | +| `parseJSON` | `parseJSON(v: dyn) -> dyn` | Parse a JSON string; returns `{}` on null/empty/invalid input. | +| `parseYAML` | `parseYAML(v: dyn) -> dyn` | Parse a YAML string; returns `{}` on null/empty/invalid input. | +| `jsonPath` | `jsonPath(root: dyn, expr: string) -> list` | Apply a JSONPath-lite subset (`$.a.b[*]`, `$..name`, `$[0]`, `$['key']`). Use when the path is dynamic. | + +#### HTTP shortcuts + +| Function | Signature | Description | +| --------- | --------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | +| `has` | `has(name: string, bucket: map) -> bool` | True if `name` is a key in `bucket` AND has a non-empty value. Works for `headers` and `queries` of request and response. | +| `firstOr` | `firstOr(bucket: map, name: string, default: string) -> string` | First value for `name` (string or list bucket), or `default` when missing/empty. | + +A handful of common idioms become very short: + +```cel +has('x-api-key', request.headers) +firstOr(request.header, 'x-debug', 'no') == 'yes' +request.query['debug'] == '1' +request.headers['x-forwarded-for'].exists(v, inCIDR(v, ['10.0.0.0/8'])) +``` -## Logging +### Logging -`request-validator` emits one structured log record per request. It -also logs internal events (boot, reload, fact fetch failures) through -the same logger. +`request-validator` emits one structured log record per evaluation, plus internal events (boot, reload, fact fetch failures) through the same logger. extAuthz logs the verdict it produced; extProc logs the mutations it applied (or, under dry-run, would have applied). ```yaml logging: - level: info # debug | info | warn | error - format: json # json | console - logBody: false # opt-in: include the request body - redactReveal: 6 # leading chars kept when masking a value - excludeHeaders: # never appear in the log + level: info # debug | info | warn | error + format: json # json | console + logBody: false # opt-in: include the request body + redactReveal: 6 # leading chars kept when masking a value + excludeHeaders: # never appear in the log - cookie - set-cookie - redactHeaders: # appear with their value masked + redactHeaders: # appear with their value masked - authorization - proxy-authorization - x-api-key - x-auth-token - redactQueryParams: # same treatment for query params + redactQueryParams: # same treatment for query params - access_token - id_token - code ``` -The whole `logging` block is optional with sensible defaults. The CLI -flags `--log-level` and `--log-format` override the file when set, -so you can crank up verbosity at runtime without editing the -ConfigMap. +The whole `logging` block is optional with sensible defaults. The CLI flags `--log-level` and `--log-format` override the file when set, so you can crank up verbosity at runtime without editing the ConfigMap. -A typical allow line in JSON: +A typical extAuthz allow line in JSON: ```json { @@ -350,87 +790,18 @@ A typical allow line in JSON: Notes on the redaction: -- `cookie` is configured as excluded, so it doesn't appear at all. -- `authorization` is long enough that we keep the first 6 characters - visible (here `Bearer`) and mask the rest with `*`. +- `cookie` is configured as excluded, so it does not appear at all. +- `authorization` is long enough that we keep the first 6 characters visible (here `Bearer`) and mask the rest with `*`. - `x-api-key` is short, so it gets fully masked. -- The query parameter `code` is in `redactQueryParams`, so it shows - as `code=***`. `debug=1` stays untouched. - -Values shorter than `2 * redactReveal` are always fully masked so -short tokens don't leak half of themselves. - -The `console` format produces the same information laid out as a -single dense `key=value` line. It is meant for `kubectl logs -f` -during development, not for ingestion. Use `json` in production. +- The query parameter `code` is in `redactQueryParams`, so it shows as `code=***`. `debug=1` stays untouched. -## CEL function reference +Values shorter than `2 * redactReveal` are always fully masked so short tokens do not leak half of themselves. -CEL itself comes with a small standard library; we enable -`ext.Strings()`, `ext.Encoders()`, `ext.Lists()`, `ext.Sets()`, -`ext.Math()` and `ext.Bindings()` on top of that. The following -project-specific functions are also registered. +The `console` format produces the same information laid out as a single dense `key=value` line. It is meant for `kubectl logs -f` during development, not for ingestion. Use `json` in production. -### Network - -| Function | Signature | Description | -| -------------- | ------------------------------------------------- | ----------------------------------------------------------------------------------------------- | -| `inCIDR` | `inCIDR(ip: string, cidrs: list) -> bool` | True if `ip` belongs to any of the listed CIDRs. Plain IPs are accepted (auto-`/32` or `/128`). | -| `ipFamily` | `ipFamily(ip: string) -> string` | `"ipv4"`, `"ipv6"` or `""`. | -| `isPrivateIP` | `isPrivateIP(ip: string) -> bool` | RFC1918, RFC4193, link-local. | -| `isLoopbackIP` | `isLoopbackIP(ip: string) -> bool` | `127.0.0.0/8`, `::1`. | -| `parseURL` | `parseURL(s: string) -> map` | Returns `{scheme, host, port, path, query, fragment, username, password}`. | - -### Strings (glob) - -| Function | Signature | Description | -| --------- | ---------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------- | -| `glob` | `glob(s: string, pattern: string) -> bool` | Shell-style globs. `*` matches anything except `/`, `**` matches everything, `?` is one char, `[abc]` is a class. | -| `globAny` | `globAny(s: string, patterns: list) -> bool` | True if any glob in the list matches. | - -For substring, replace, split, lower, upper and similar, use CEL's -`ext.Strings()` directly: `s.lower()`, `s.split(',')`, etc. - -### Encoding and hashing - -| Function | Signature | Description | -| -------------------- | ------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------ | -| `sha256Hex` | `sha256Hex(s: string) -> string` | Lowercase hex of SHA-256(s). | -| `parseJWTUnverified` | `parseJWTUnverified(token: string) -> map` | Returns `{header, payload}` parsed JSON. **Does not** verify the signature; use only when another component already did. | - -`base64.encode` and `base64.decode` come from `ext.Encoders()`. +## Operating the service -### Time - -| Function | Signature | Description | -| -------- | -------------------- | ---------------------------------------------------------------------- | -| `now` | `now() -> timestamp` | Current UTC time. CEL accessors (`getHours`, `getDayOfWeek`...) apply. | - -### Structured data - -| Function | Signature | Description | -| ----------- | ------------------------------------------------ | ------------------------------------------------------------------------------------------------------- | -| `parseJSON` | `parseJSON(v: dyn) -> dyn` | Parse a JSON string; returns `{}` on null/empty/invalid input. | -| `parseYAML` | `parseYAML(v: dyn) -> dyn` | Parse a YAML string; returns `{}` on null/empty/invalid input. | -| `jsonPath` | `jsonPath(root: dyn, expr: string) -> list` | Apply a JSONPath-lite subset (`$.a.b[*]`, `$..name`, `$[0]`, `$['key']`). Use when the path is dynamic. | - -### HTTP shortcuts - -| Function | Signature | Description | -| --------- | --------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | -| `has` | `has(name: string, bucket: map) -> bool` | True if `name` is a key in `bucket` AND has at least one non-empty value. Works for `request.headers` and `request.queries`. | -| `firstOr` | `firstOr(bucket: map, name: string, default: string) -> string` | First value for `name` (string or list bucket), or `default` when missing/empty. | - -A handful of common idioms become very short: - -```cel -has('x-api-key', request.headers) -firstOr(request.header, 'x-debug', 'no') == 'yes' -request.query['debug'] == '1' -request.headers['x-forwarded-for'].exists(v, inCIDR(v, ['10.0.0.0/8'])) -``` - -## Running it +### Running it From source, with the bundled example: @@ -438,11 +809,7 @@ From source, with the bundled example: go run ./cmd --config examples/policy.yaml --log-level debug --log-format console ``` -The project also ships an OCI image at -`ghcr.io/achetronic/request-validator:`. Deploy it with -whatever templating you already use. We use -[`bjw-s` app-template](https://github.com/bjw-s-labs/helm-charts/tree/main/charts/other/app-template); -a minimal HelmRelease values block looks like: +The project also ships an OCI image at `ghcr.io/achetronic/request-validator:`. Deploy it with whatever templating you already use. We use [`bjw-s` app-template](https://github.com/bjw-s-labs/helm-charts/tree/main/charts/other/app-template); a minimal HelmRelease values block looks like: ```yaml controllers: @@ -455,50 +822,59 @@ controllers: args: - --config=/etc/policy/policy.yaml probes: - liveness: { type: HTTP, custom: true, spec: { httpGet: { path: /healthz, port: 8080 } } } - readiness: { type: HTTP, custom: true, spec: { httpGet: { path: /readyz, port: 8080 } } } + liveness: + { + type: HTTP, + custom: true, + spec: { httpGet: { path: /healthz, port: 8080 } }, + } + readiness: + { + type: HTTP, + custom: true, + spec: { httpGet: { path: /readyz, port: 8080 } }, + } service: - main: { controller: main, ports: { http: { port: 8080 } } } + # If you use the extProc engine, the gRPC port name MUST start with + # `grpc-` or `http2-` so Istio negotiates HTTP/2 on its cluster. + main: { controller: main, ports: { http: { port: 8080 }, grpc-extproc: { port: 9090 } } } persistence: policy: type: configMap name: request-validator-policy - globalMounts: [ { path: /etc/policy } ] + globalMounts: [{ path: /etc/policy }] ``` ### Endpoints -| Endpoint | Purpose | -| ---------- | ---------------------------------------------------- | -| `/` | ext-authz endpoint; everything else lands here | -| `/healthz` | liveness | -| `/readyz` | readiness; false until a policy is loaded | -| `/metrics` | Prometheus counters, broken down by group and rule | +The HTTP server (`--port`, default 8080) hosts the extAuthz engine and the operational endpoints: + +| Endpoint | Purpose | +| ---------- | -------------------------------------------------------------------- | +| `/` | extAuthz check; Envoy posts the request here, everything lands here | +| `/healthz` | Liveness probe | +| `/readyz` | Readiness probe; false until a policy is successfully loaded | +| `/metrics` | Prometheus counters, broken down by group and rule | + +The gRPC server (`--grpc-port`, default 9090) hosts the extProc engine: it serves Envoy's `ext_proc` `ExternalProcessor` service. Both servers share the same policy and reload together. ### Hot reload -The policy file is reloaded automatically as soon as it changes on -disk. This covers three realistic scenarios: +The policy file is reloaded automatically as soon as it changes on disk, for both engines at once. This covers three realistic scenarios: - A plain in-place write picks up immediately. -- A save-via-rename (vim's `:w`, IntelliJ's atomic write, etc.) is - also recognised. -- Kubernetes ConfigMap updates trigger an in-process reload too: the - watcher tracks the `..data` symlink kubelet flips atomically when - it publishes a new projection. +- A save-via-rename (vim's `:w`, IntelliJ's atomic write, etc.) is also recognised. +- Kubernetes ConfigMap updates trigger an in-process reload too: the watcher tracks the `..data` symlink kubelet flips atomically when it publishes a new projection. -Multiple events within a short window (default 200 ms) are debounced -into a single reload. If the new policy fails to load (parse error, -fact fetch failure, etc.) it is rejected and the previous policy -stays active. +Multiple events within a short window (default 200 ms) are debounced into a single reload. If the new policy fails to load (parse error, fact fetch failure, etc.) it is rejected and the previous policy stays active. -If fsnotify can't deliver events (NFS, FUSE), sending `SIGHUP` -to the process triggers the same reload code path. +If fsnotify cannot deliver events (NFS, FUSE), sending `SIGHUP` to the process triggers the same reload code path. ### Flags ``` ---port HTTP port (default 8080) +--port HTTP port for the extAuthz engine (default 8080) +--grpc-port gRPC port for the extProc engine (default 9090) --config Path to the YAML policy (default policy.yaml) --log-level Override logging.level (debug|info|warn|error) --log-format Override logging.format (json|console) @@ -507,31 +883,28 @@ to the process triggers the same reload code path. --version Print version and exit ``` -The config file is run through `os.ExpandEnv` before parsing, so you -can use `$VAR` and `${VAR}` placeholders that get substituted from -environment variables. +The config file is run through `os.ExpandEnv` before parsing, so you can use `$VAR` and `${VAR}` placeholders that get substituted from environment variables. -## Response headers +### Response headers -Every response carries a few diagnostic headers so you can see which -rule produced the verdict: +The extAuthz engine stamps a few diagnostic headers on its response so you can see which rule produced the verdict (extProc instead rewrites traffic per your policy and does not add these): -| Header | Value | -| -------------- | ----------------------------------------------------------- | -| `x-rv-result` | `allow` or `deny` | -| `x-rv-rule` | rule that decided, formatted `group/rule` (or ``) | -| `x-rv-reason` | short, human-readable reason | +| Header | Value | +| -------------- | ------------------------------------------------------------------------------------------------------- | +| `x-rv-result` | `allow` or `deny` | +| `x-rv-rule` | rule that decided, formatted `group/rule` (or ``) | +| `x-rv-reason` | short, human-readable reason | | `x-rv-dry-run` | `true` when enforcement was suppressed, by the deciding rule's `dryRun` or the global `defaults.dryRun` | -Useful both during development and as a way for the protected service -to log "this request was let through by rule X" if it wants to. +Useful during development and as a way for the protected service to log "this request was let through by rule X" if it wants to. + +### Plugging into Istio -## Plugging into Istio +The two engines wire into Istio differently. Full, copy-ready manifests live in [`examples/config-for-istio-extauthz.yaml`](examples/config-for-istio-extauthz.yaml) and [`examples/config-for-istio-extproc.yaml`](examples/config-for-istio-extproc.yaml). The shape of each: -Two pieces of Istio configuration: +**extAuthz** uses a MeshConfig `extensionProvider` plus an `AuthorizationPolicy`: -1. Register the validator as an extension provider in `MeshConfig`. - This is mesh-wide; one entry per validator deployment. +1. Register the validator as an HTTP ext-authz provider in `MeshConfig` (mesh-wide): ```yaml # Ref: https://istio.io/latest/docs/reference/config/istio.mesh.v1alpha1/ @@ -543,27 +916,16 @@ Two pieces of Istio configuration: port: 8080 failOpen: false timeout: 2s - # Forward the body so policies can inspect it. Only - # maxRequestBytes and allowPartialMessage apply here; - # packAsBytes is gRPC-only. includeRequestBodyInCheck: maxRequestBytes: 1048576 allowPartialMessage: false - headersToDownstreamOnDeny: [content-type, x-rv-result, x-rv-rule, x-rv-reason, x-rv-dry-run] - headersToUpstreamOnAllow: [x-rv-result, x-rv-rule, x-rv-reason, x-rv-dry-run] - includeRequestHeadersInCheck: - - authorization - - content-type - - cookie - - x-api-key - - x-user-groups - - x-forwarded-for - - x-forwarded-proto + headersToDownstreamOnDeny: + [content-type, x-rv-result, x-rv-rule, x-rv-reason, x-rv-dry-run] + headersToUpstreamOnAllow: + [x-rv-result, x-rv-rule, x-rv-reason, x-rv-dry-run] ``` -2. Point an `AuthorizationPolicy` with `action: CUSTOM` at it. Only - the matched traffic is delegated to the validator; the rest stays - on whatever Istio policies you had before. +2. Point an `AuthorizationPolicy` with `action: CUSTOM` at that provider, scoped to the traffic you want gated: ```yaml apiVersion: security.istio.io/v1 @@ -587,12 +949,9 @@ Two pieces of Istio configuration: - /realms/*/clients-registrations/* ``` -If you find yourself adding to `includeRequestHeadersInCheck` every -time a policy starts looking at a new header, there is a more -flexible alternative: an `EnvoyFilter` that pins the ext-authz -filter's `allowed_headers` to `.*` on the protected workload's -sidecar. See [`examples/config-for-istio.yaml`](examples/config-for-istio.yaml) -for the full pattern. +**extProc** has no MeshConfig extensionProvider in Istio, so it is wired with an `EnvoyFilter` that inserts the `ext_proc` HTTP filter on the target workload's sidecar, pointing at the validator's gRPC port. The gRPC port on the validator Service must be named `grpc-*` or `http2-*` for Istio to negotiate HTTP/2. See [`examples/config-for-istio-extproc.yaml`](examples/config-for-istio-extproc.yaml) for the full filter, including the `processing_mode` block that selects which phases Envoy streams to the validator. + +If you find yourself adding to `includeRequestHeadersInCheck` every time a policy starts reading a new header, the extAuthz example also shows a more flexible alternative: an `EnvoyFilter` that pins the ext-authz filter's `allowed_headers` to `.*` on the workload's sidecar. ## License diff --git a/cmd/main.go b/cmd/main.go index 92718c9..d8e5efb 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -15,6 +15,7 @@ import ( "time" "request-validator/internal/configwatch" + "request-validator/internal/grpcserver" "request-validator/internal/httpserver" "request-validator/internal/log" "request-validator/internal/policy" @@ -26,6 +27,7 @@ var version = "dev" func main() { var ( port = flag.Int("port", 8080, "HTTP server port") + grpcPort = flag.Int("grpc-port", 9090, "gRPC ext_proc server port") configPath = flag.String("config", "policy.yaml", "Path to the policy file") level = flag.String("log-level", "", "Override logging.level from the policy file (debug|info|warn|error)") format = flag.String("log-format", "", "Override logging.format from the policy file (json|console)") @@ -54,6 +56,7 @@ func main() { logLoaded("policy loaded", *configPath, cfg) srv := httpserver.New(cfg) + gsrv := grpcserver.New(cfg) // Single function used by every reload trigger (SIGHUP and fsnotify). // The new config replaces the old one atomically; the old facts @@ -73,6 +76,7 @@ func main() { } applyLogging(newCfg.Logging, *level, *format) old := srv.SetPolicy(newCfg) + gsrv.SetPolicy(newCfg) if old != nil { old.Stop() } @@ -109,13 +113,15 @@ func main() { stop := make(chan os.Signal, 1) signal.Notify(stop, syscall.SIGINT, syscall.SIGTERM) - errCh := make(chan error, 1) + errCh := make(chan error, 2) go func() { errCh <- srv.Run(fmt.Sprintf(":%d", *port)) }() + go func() { errCh <- gsrv.Run(fmt.Sprintf(":%d", *grpcPort)) }() select { case <-stop: log.Infow("shutting down") srv.Stop() + gsrv.Stop() // Stop the shared-source goroutines belonging to the policy that // was current at shutdown time. if p := srv.Policy(); p != nil { @@ -141,7 +147,7 @@ func logLoaded(msg, path string, cfg *policy.Config, extra ...any) { "path", path, "groups", len(cfg.Groups), "rules", rules, - "default", cfg.Defaults.Action, + "default", cfg.Defaults.ExtAuthz.Action, } kv = append(kv, extra...) log.Infow(msg, kv...) diff --git a/examples/config-for-istio.yaml b/examples/config-for-istio-extauthz.yaml similarity index 97% rename from examples/config-for-istio.yaml rename to examples/config-for-istio-extauthz.yaml index ddd31a6..b12cde0 100644 --- a/examples/config-for-istio.yaml +++ b/examples/config-for-istio-extauthz.yaml @@ -1,4 +1,7 @@ -# Istio wiring for request-validator. +# Istio wiring for the request-validator extAuthz engine (HTTP, :8080). +# +# The extProc engine (gRPC) is wired separately, see +# config-for-istio-extproc.yaml. # # Three pieces fit together below: # diff --git a/examples/config-for-istio-extproc.yaml b/examples/config-for-istio-extproc.yaml new file mode 100644 index 0000000..f56ed10 --- /dev/null +++ b/examples/config-for-istio-extproc.yaml @@ -0,0 +1,82 @@ +# Istio wiring for the request-validator extProc engine (gRPC, :9090). +# +# The extAuthz engine (HTTP) is wired separately, see +# config-for-istio-extauthz.yaml. +# +# Istio has no MeshConfig extensionProvider for ext_proc, so the whole +# wiring is a single EnvoyFilter that inserts the ext_proc HTTP filter on +# the protected workload's inbound sidecar. The response the workload +# produces (e.g. Keycloak's 302 during DCR) flows back out through this +# same HTTP connection manager, so the responseHeaders / responseBody +# phases fire here and the validator can rewrite it (the interstitial +# recipe in policy.yaml). +# +# Replace the placeholders before applying: +# namespace where request-validator runs. +# namespace where the workload being protected runs +# (e.g. the Keycloak namespace). +# app.kubernetes.io/name (or equivalent) label of +# the workload whose sidecar gets the filter. +# +# Prerequisite on the validator Service: the gRPC port (9090) MUST be +# named with a `grpc-` or `http2-` prefix (e.g. `grpc-extproc`). Istio +# only negotiates HTTP/2 on the generated cluster when the port name says +# so, and gRPC needs HTTP/2. A plain `tcp`/`http` name leaves the cluster +# on HTTP/1 and the ext_proc stream never connects. + + +apiVersion: networking.istio.io/v1alpha3 +kind: EnvoyFilter +metadata: + name: request-validator-ext-proc + namespace: +spec: + workloadSelector: + labels: + app.kubernetes.io/name: + configPatches: + - applyTo: HTTP_FILTER + match: + context: SIDECAR_INBOUND + listener: + filterChain: + filter: + name: envoy.filters.network.http_connection_manager + subFilter: + # Insert ext_proc right before the router so it runs after + # ext_authz has already gated the request and still sees + # the full response on the way out. + name: envoy.filters.http.router + patch: + operation: INSERT_BEFORE + value: + name: envoy.filters.http.ext_proc + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.ext_proc.v3.ExternalProcessor + # Point at the cluster Istio already generates for the + # validator Service. No hand-written cluster needed. + grpc_service: + envoy_grpc: + cluster_name: outbound|9090||request-validator..svc.cluster.local + timeout: 2s + # How long Envoy waits for the validator to answer a single + # ProcessingRequest before giving up on it. + message_timeout: 2s + # Fail closed, mirroring failOpen: false on the ext-authz + # provider: if the validator is unreachable the response is not + # served. Set true to fail open, which lets untrusted redirects + # through unwarned when the validator is down. + failure_mode_allow: false + # Match the phases the policy actually uses. This default fits + # the interstitial recipe: skip the request, inspect response + # headers, touch no bodies. Widen as the policy grows: + # - extProc requestHeaders groups need request_header_mode: SEND. + # - *Body phases (setBody, or directResponse reading + # response.body) need the matching body mode set to BUFFERED, + # with the buffer kept in step with + # defaults.extProc.maxBodyBytes in the policy. + processing_mode: + request_header_mode: SKIP + response_header_mode: SEND + request_body_mode: NONE + response_body_mode: NONE diff --git a/examples/interstitial.html b/examples/interstitial.html new file mode 100644 index 0000000..68b9144 --- /dev/null +++ b/examples/interstitial.html @@ -0,0 +1,17 @@ + + + + + Untrusted Redirect Warning + + +

Warning: Untrusted Redirect

+

The application is trying to redirect you to an untrusted external destination.

+

If you wish to proceed, click the link below:

+ Go to external place + + + diff --git a/examples/policy.yaml b/examples/policy.yaml index e51dee2..71eb630 100644 --- a/examples/policy.yaml +++ b/examples/policy.yaml @@ -1,5 +1,4 @@ -# Example policy for the Keycloak DCR (Dynamic Client Registration) endpoint -# on auth.example-1.com / auth.example-2.com, restricted to the `mcp` realm. +# on auth.example-1.com / auth.example-2.com, restricted to the mcp realm. # The corporate Keycloak hostname is always denied. # # Trust model summary: @@ -21,23 +20,24 @@ # Engine-wide settings. What happens when no group decides, how big a body # we are willing to read, and whether evaluation errors should fail open. defaults: - action: deny - denyStatus: 403 - denyBody: "Forbidden" - maxBodyBytes: 1MiB - allowOnError: false + extAuthz: + action: deny + denyStatus: 403 + denyBody: "Forbidden" + maxBodyBytes: 1MiB + allowOnError: false dryRun: false # global shadow mode: evaluate, never enforce # Logging produces one structured access-log record per request, plus # anything the engine wants to say (boot, reload, fact fetch failures, # etc.). The CLI flags --log-level and --log-format, when set, override -# the values in this section. +# the file values in this section. # -# Headers in `excludeHeaders` never appear in the log. Headers in -# `redactHeaders` appear but with their value masked: the first -# `redactReveal` characters are shown, the rest is replaced with '*'. -# Values shorter than `2 * redactReveal` are fully masked, so short +# Headers in excludeHeaders never appear in the log. Headers in +# redactHeaders appear but with their value masked: the first +# redactReveal characters are shown, the rest is replaced with '*'. +# Values shorter than 2 * redactReveal are fully masked, so short # tokens don't leak by accident. logging: level: info # debug | info | warn | error @@ -58,7 +58,7 @@ logging: - code -# Named external values referenced as `facts.` from CEL. +# Named external values referenced as facts. from CEL. # # Three ways to load each one: # value inline literal (any YAML shape). @@ -109,16 +109,25 @@ facts: interval: 10m timeout: 15s + # Domains trusted for DCR redirects. + - name: trustedRedirectDomains + method: value + value: + - example-1.com + - example-2.com + - google.com + + # Interstitial HTML warning page content. + - name: interstitialHtml + method: file + file: + path: examples/interstitial.html + -# Ordered list of rule buckets. The first group whose `match` is true and + +# Ordered list of rule buckets. The first group whose match is true and # whose internal rules produce a verdict wins; otherwise defaults.action # applies. -# -# Per-group `mode`: -# firstMatch (default): the first rule whose `match` is true decides. -# all: every rule must match, otherwise the group denies. -# -# A rule inherits its `action` from the group unless it declares its own. groups: # NOTE on Keycloak DCR paths. @@ -127,20 +136,26 @@ groups: # The regex `^/realms/[^/]+/clients-registrations(/.*)?$` covers both the # bare collection path and the `/openid-connect[/...]` sub-paths. The # paired Istio AuthorizationPolicy needs glob patterns that span slashes - # too - see examples/config-for-istio.yaml for the `{*}` / `{**}` syntax. + # too - see examples/config-for-istio-extauthz.yaml for the `{*}`, `{**}` syntax. # Corporate Keycloak: DCR is never allowed here. - name: dcr-corporate-deny - action: deny + parameters: + engine: extAuthz + mode: firstMatch rules: - name: block-corporate match: | request.host == 'keycloak.internal.example-1.com' && request.path.matches('^/realms/[^/]+/clients-registrations(/.*)?$') + validation: + action: deny # Anything coming from the internal network on the mcp realm gets in. - name: dcr-internal - action: allow + parameters: + engine: extAuthz + mode: firstMatch match: | request.method == 'POST' && request.host in ['auth.example-2.com', 'auth.example-1.com'] && @@ -148,10 +163,14 @@ groups: rules: - name: from-internal-cidr match: inCIDR(request.remoteIp, facts.internalCidrs) + validation: + action: allow # Claude calls in from a small, stable set of CIDRs. - name: dcr-claude - action: allow + parameters: + engine: extAuthz + mode: firstMatch match: | request.method == 'POST' && request.host in ['auth.example-2.com', 'auth.example-1.com'] && @@ -159,12 +178,16 @@ groups: rules: - name: from-claude-cidr match: inCIDR(request.remoteIp, facts.claudeCidrs) + validation: + action: allow # ChatGPT calls in from the CIDRs OpenAI publishes. The scope is guarded # so the rule simply doesn't fire if the feed hasn't been populated yet # (tests, or the few seconds before the first successful fetch on boot). - name: dcr-chatgpt - action: allow + parameters: + engine: extAuthz + mode: firstMatch match: | request.method == 'POST' && request.host in ['auth.example-2.com', 'auth.example-1.com'] && @@ -175,10 +198,14 @@ groups: match: | inCIDR(request.remoteIp, parseJSON(facts.chatgptFeed).prefixes.map(p, p.ipv4Prefix)) + validation: + action: allow # Mistral, in dry-run for now. Same shape as the ChatGPT rule. - name: dcr-mistral - action: allow + parameters: + engine: extAuthz + mode: firstMatch match: | request.method == 'POST' && request.host in ['auth.example-2.com', 'auth.example-1.com'] && @@ -190,6 +217,8 @@ groups: match: | inCIDR(request.remoteIp, parseJSON(facts.mistralFeed).prefixes.map(p, p.ipv4Prefix)) + validation: + action: allow # Antigravity runs on the end user's machine, so the source IP is # whatever residential ISP they're on. We have to recognise the client @@ -203,7 +232,9 @@ groups: # Both are fixed properties of the Antigravity client, so they live in # the rule itself rather than as facts. - name: dcr-antigravity - action: allow + parameters: + engine: extAuthz + mode: firstMatch match: | request.method == 'POST' && request.host in ['auth.example-2.com', 'auth.example-1.com'] && @@ -213,8 +244,9 @@ groups: rules: # Sanity cap: more than 5 redirect URIs is fishy regardless of shape. - name: too-many-redirects - action: deny match: "size(request.body.json.redirect_uris) > 5" + validation: + action: deny # The canonical Google-hosted callback, used when OAuth is set up # manually instead of through DCR. Exactly one URI, fixed value. @@ -222,6 +254,8 @@ groups: match: | size(request.body.json.redirect_uris) == 1 && request.body.json.redirect_uris[0] == 'https://antigravity.google/oauth-callback' + validation: + action: allow # DCR-issued loopback clients. Every redirect_uri must be HTTPS, # loopback, with a non-empty port. We also sanity-check the @@ -238,13 +272,58 @@ groups: request.body.json.response_types == ['code']) && (!has(request.body.json.token_endpoint_auth_method) || request.body.json.token_endpoint_auth_method == 'none') + validation: + action: allow # The master realm is for administrators only. Nobody should reach it # through the public hostname. - name: block-master-realm-public - action: deny + parameters: + engine: extAuthz + mode: firstMatch rules: - name: block match: | request.host in ['auth.example-2.com', 'auth.example-1.com'] && request.path.startsWith('/realms/master') + validation: + action: deny + + # Intercept Keycloak redirects during DCR. If the target is untrusted, + # serve our warning interstitial page directly. + - name: dcr-redirect-rewriter + parameters: + engine: extProc + phase: responseHeaders + mode: firstMatch + match: | + response.status == 302 && + has('location', response.headers) + rules: + - name: redirect-to-interstitial + match: | + !facts.trustedRedirectDomains.exists(d, + parseURL(response.header['location']).host == d || + parseURL(response.header['location']).host.endsWith('.' + d) + ) + mutations: + - op: directResponse + status: 200 + headers: '{"content-type": "text/html"}' + body: "facts.interstitialHtml.replace('__TARGET_B64__', base64.encode(response.header['location']))" + + # Strip sensitive internal headers from all upstream responses. + - name: downstream-header-stripper + parameters: + engine: extProc + phase: responseHeaders + mode: applyAll + rules: + - name: strip-internal-headers + match: "true" + mutations: + - op: removeHeader + name: Server + - op: removeHeader + name: X-Powered-By + diff --git a/go.mod b/go.mod index cff2439..7bba914 100644 --- a/go.mod +++ b/go.mod @@ -1,19 +1,30 @@ module request-validator -go 1.23.0 +go 1.24.0 -require github.com/google/cel-go v0.28.1 +require ( + github.com/envoyproxy/go-control-plane/envoy v1.37.0 + github.com/fsnotify/fsnotify v1.10.1 + github.com/google/cel-go v0.28.1 + google.golang.org/grpc v1.78.0 + gopkg.in/yaml.v3 v3.0.1 +) require ( cel.dev/expr v0.25.1 // indirect github.com/antlr4-go/antlr/v4 v4.13.1 // indirect - github.com/fsnotify/fsnotify v1.10.1 // indirect + github.com/cncf/xds/go v0.0.0-20251110193048-8bfbf64dc13e // indirect + github.com/envoyproxy/protoc-gen-validate v1.3.0 // indirect + github.com/kr/pretty v0.3.1 // indirect + github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect + github.com/rogpeppe/go-internal v1.12.0 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/exp v0.0.0-20240823005443-9b4947da3948 // indirect - golang.org/x/sys v0.21.0 // indirect - golang.org/x/text v0.22.0 // indirect - google.golang.org/genproto/googleapis/api v0.0.0-20240826202546-f6391c0de4c7 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20240826202546-f6391c0de4c7 // indirect - google.golang.org/protobuf v1.36.10 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect + golang.org/x/net v0.47.0 // indirect + golang.org/x/sys v0.38.0 // indirect + golang.org/x/text v0.31.0 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20251029180050-ab9386a59fda // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20251029180050-ab9386a59fda // indirect + google.golang.org/protobuf v1.36.11 // indirect + gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect ) diff --git a/go.sum b/go.sum index 1f83ae3..137de53 100644 --- a/go.sum +++ b/go.sum @@ -2,27 +2,74 @@ cel.dev/expr v0.25.1 h1:1KrZg61W6TWSxuNZ37Xy49ps13NUovb66QLprthtwi4= cel.dev/expr v0.25.1/go.mod h1:hrXvqGP6G6gyx8UAHSHJ5RGk//1Oj5nXQ2NI02Nrsg4= github.com/antlr4-go/antlr/v4 v4.13.1 h1:SqQKkuVZ+zWkMMNkjy5FZe5mr5WURWnlpmOuzYWrPrQ= github.com/antlr4-go/antlr/v4 v4.13.1/go.mod h1:GKmUxMtwp6ZgGwZSva4eWPC5mS6vUAmOABFgjdkM7Nw= +github.com/cncf/xds/go v0.0.0-20251110193048-8bfbf64dc13e h1:gt7U1Igw0xbJdyaCM5H2CnlAlPSkzrhsebQB6WQWjLA= +github.com/cncf/xds/go v0.0.0-20251110193048-8bfbf64dc13e/go.mod h1:KdCmV+x/BuvyMxRnYBlmVaq4OLiKW6iRQfvC62cvdkI= +github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= +github.com/envoyproxy/go-control-plane/envoy v1.37.0 h1:u3riX6BoYRfF4Dr7dwSOroNfdSbEPe9Yyl09/B6wBrQ= +github.com/envoyproxy/go-control-plane/envoy v1.37.0/go.mod h1:DReE9MMrmecPy+YvQOAOHNYMALuowAnbjjEMkkWOi6A= +github.com/envoyproxy/protoc-gen-validate v1.3.0 h1:TvGH1wof4H33rezVKWSpqKz5NXWg5VPuZ0uONDT6eb4= +github.com/envoyproxy/protoc-gen-validate v1.3.0/go.mod h1:HvYl7zwPa5mffgyeTUHA9zHIH36nmrm7oCbo4YKoSWA= github.com/fsnotify/fsnotify v1.10.1 h1:b0/UzAf9yR5rhf3RPm9gf3ehBPpf0oZKIjtpKrx59Ho= github.com/fsnotify/fsnotify v1.10.1/go.mod h1:TLheqan6HD6GBK6PrDWyDPBaEV8LspOxvPSjC+bVfgo= +github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= +github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= +github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= +github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= +github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/cel-go v0.28.1 h1:YWIwi77J4xIsYUwAF/iIuS6haffzIHS8yWI8glSbLWM= github.com/google/cel-go v0.28.1/go.mod h1:X0bD6iVNR8pkROSOoHVdgTkzmRcosof7WQqCD6wcMc8= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= +github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= +github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo= +github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10/go.mod h1:t/avpk3KcrXxUnYOhZhMXJlSEyie6gQbtLq5NM3loB8= +github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= +github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8= +github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4= +go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= +go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= +go.opentelemetry.io/otel v1.38.0 h1:RkfdswUDRimDg0m2Az18RKOsnI8UDzppJAtj01/Ymk8= +go.opentelemetry.io/otel v1.38.0/go.mod h1:zcmtmQ1+YmQM9wrNsTGV/q/uyusom3P8RxwExxkZhjM= +go.opentelemetry.io/otel/metric v1.38.0 h1:Kl6lzIYGAh5M159u9NgiRkmoMKjvbsKtYRwgfrA6WpA= +go.opentelemetry.io/otel/metric v1.38.0/go.mod h1:kB5n/QoRM8YwmUahxvI3bO34eVtQf2i4utNVLr9gEmI= +go.opentelemetry.io/otel/sdk v1.38.0 h1:l48sr5YbNf2hpCUj/FoGhW9yDkl+Ma+LrVl8qaM5b+E= +go.opentelemetry.io/otel/sdk v1.38.0/go.mod h1:ghmNdGlVemJI3+ZB5iDEuk4bWA3GkTpW+DOoZMYBVVg= +go.opentelemetry.io/otel/sdk/metric v1.38.0 h1:aSH66iL0aZqo//xXzQLYozmWrXxyFkBJ6qT5wthqPoM= +go.opentelemetry.io/otel/sdk/metric v1.38.0/go.mod h1:dg9PBnW9XdQ1Hd6ZnRz689CbtrUp0wMMs9iPcgT9EZA= +go.opentelemetry.io/otel/trace v1.38.0 h1:Fxk5bKrDZJUH+AMyyIXGcFAPah0oRcT+LuNtJrmcNLE= +go.opentelemetry.io/otel/trace v1.38.0/go.mod h1:j1P9ivuFsTceSWe1oY+EeW3sc+Pp42sO++GHkg4wwhs= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/exp v0.0.0-20240823005443-9b4947da3948 h1:kx6Ds3MlpiUHKj7syVnbp57++8WpuKPcR5yjLBjvLEA= golang.org/x/exp v0.0.0-20240823005443-9b4947da3948/go.mod h1:akd2r19cwCdwSwWeIdzYQGa/EZZyqcOdwWiwj5L5eKQ= -golang.org/x/sys v0.21.0 h1:rF+pYz3DAGSQAxAu1CbC7catZg4ebC4UIeIhKxBZvws= -golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/text v0.22.0 h1:bofq7m3/HAFvbF51jz3Q9wLg3jkvSPuiZu/pD1XwgtM= -golang.org/x/text v0.22.0/go.mod h1:YRoo4H8PVmsu+E3Ou7cqLVH8oXWIHVoX0jqUWALQhfY= -google.golang.org/genproto/googleapis/api v0.0.0-20240826202546-f6391c0de4c7 h1:YcyjlL1PRr2Q17/I0dPk2JmYS5CDXfcdb2Z3YRioEbw= -google.golang.org/genproto/googleapis/api v0.0.0-20240826202546-f6391c0de4c7/go.mod h1:OCdP9MfskevB/rbYvHTsXTtKC+3bHWajPdoKgjcYkfo= -google.golang.org/genproto/googleapis/rpc v0.0.0-20240826202546-f6391c0de4c7 h1:2035KHhUv+EpyB+hWgJnaWKJOdX1E95w2S8Rr4uWKTs= -google.golang.org/genproto/googleapis/rpc v0.0.0-20240826202546-f6391c0de4c7/go.mod h1:UqMtugtsSgubUsoxbuAoiCXvqvErP7Gf0so0mK9tHxU= -google.golang.org/protobuf v1.36.10 h1:AYd7cD/uASjIL6Q9LiTjz8JLcrh/88q5UObnmY3aOOE= -google.golang.org/protobuf v1.36.10/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +golang.org/x/net v0.47.0 h1:Mx+4dIFzqraBXUugkia1OOvlD6LemFo1ALMHjrXDOhY= +golang.org/x/net v0.47.0/go.mod h1:/jNxtkgq5yWUGYkaZGqo27cfGZ1c5Nen03aYrrKpVRU= +golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc= +golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/text v0.31.0 h1:aC8ghyu4JhP8VojJ2lEHBnochRno1sgL6nEi9WGFGMM= +golang.org/x/text v0.31.0/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM= +gonum.org/v1/gonum v0.16.0 h1:5+ul4Swaf3ESvrOnidPp4GZbzf0mxVQpDCYUQE7OJfk= +gonum.org/v1/gonum v0.16.0/go.mod h1:fef3am4MQ93R2HHpKnLk4/Tbh/s0+wqD5nfa6Pnwy4E= +google.golang.org/genproto/googleapis/api v0.0.0-20251029180050-ab9386a59fda h1:+2XxjfsAu6vqFxwGBRcHiMaDCuZiqXGDUDVWVtrFAnE= +google.golang.org/genproto/googleapis/api v0.0.0-20251029180050-ab9386a59fda/go.mod h1:fDMmzKV90WSg1NbozdqrE64fkuTv6mlq2zxo9ad+3yo= +google.golang.org/genproto/googleapis/rpc v0.0.0-20251029180050-ab9386a59fda h1:i/Q+bfisr7gq6feoJnS/DlpdwEL4ihp41fvRiM3Ork0= +google.golang.org/genproto/googleapis/rpc v0.0.0-20251029180050-ab9386a59fda/go.mod h1:7i2o+ce6H/6BluujYR+kqX3GKH+dChPTQU19wjRPiGk= +google.golang.org/grpc v1.78.0 h1:K1XZG/yGDJnzMdd/uZHAkVqJE+xIDOcmdSFZkBUicNc= +google.golang.org/grpc v1.78.0/go.mod h1:I47qjTo4OKbMkjA/aOOwxDIiPSBofUtQUI5EfpWvW7U= +google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= +google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/celenv/env.go b/internal/celenv/env.go index a372db2..2a92025 100644 --- a/internal/celenv/env.go +++ b/internal/celenv/env.go @@ -1,16 +1,11 @@ // Package celenv builds the CEL environment used by request-validator -// policies, registers the custom functions catalogued in the README and -// provides a small program cache so each unique expression is compiled -// at most once. -// -// The package owns the contract between policies and Go code. The shape of -// the `request` variable exposed to CEL is the source of truth for the -// values policies can introspect. +// policies, registers the custom functions and provides a program cache. package celenv import ( "context" "fmt" + "reflect" "sync" "github.com/google/cel-go/cel" @@ -19,66 +14,109 @@ import ( "github.com/google/cel-go/ext" ) -// Env is the compiled CEL environment plus a compiled-program cache. +// Scope defines the variable visibility scope for CEL compilation. +type Scope int + +const ( + // ScopeRequest permits access to request and facts. + ScopeRequest Scope = iota + // ScopeResponse permits access to request, response, and facts. + ScopeResponse +) + +// Env wraps the request and response CEL environments and a program cache. type Env struct { - env *cel.Env - mu sync.RWMutex - cache map[string]cel.Program + requestEnv *cel.Env + responseEnv *cel.Env + mu sync.RWMutex + cache map[cacheKey]cel.Program } -// New builds the global CEL environment with all custom functions and -// extension libraries registered. -func New() (*Env, error) { - e, err := cel.NewEnv( - // Variables visible to policies. - // - `request` carries the request being authorised. - // - `facts` is the snapshot of the facts registry - // (inline / file / url). Empty when no facts entries declared. - cel.Variable("request", cel.DynType), - cel.Variable("facts", cel.DynType), +type cacheKey struct { + scope Scope + kind string + src string +} - // Bundled CEL extensions we want to ship by default. - ext.Strings(), // s.split(sep), s.replace, s.indexOf, s.lower(), etc. - ext.Encoders(), // base64.encode, base64.decode - ext.Lists(), // .sort(), .distinct(), .flatten(), .reverse() - ext.Sets(), // sets.contains, sets.intersects, sets.equivalent - ext.Math(), // math.greatest, math.least - ext.Bindings(), // cel.bind(name, value, expr) +// New builds the CEL environments with all custom functions and extensions. +func New() (*Env, error) { + commonOpts := []cel.EnvOption{ + ext.Strings(), + ext.Encoders(), + ext.Lists(), + ext.Sets(), + ext.Math(), + ext.Bindings(), - // Our custom function catalogue. netLibrary(), stringLibrary(), encodingLibrary(), timeLibrary(), dataLibrary(), httpShortcutsLibrary(), - ) + } + + reqOpts := append([]cel.EnvOption{ + cel.Variable("request", cel.DynType), + cel.Variable("facts", cel.DynType), + }, commonOpts...) + + reqEnv, err := cel.NewEnv(reqOpts...) + if err != nil { + return nil, fmt.Errorf("build CEL request env: %w", err) + } + + respOpts := append([]cel.EnvOption{ + cel.Variable("request", cel.DynType), + cel.Variable("response", cel.DynType), + cel.Variable("facts", cel.DynType), + }, commonOpts...) + + respEnv, err := cel.NewEnv(respOpts...) if err != nil { - return nil, fmt.Errorf("build CEL env: %w", err) + return nil, fmt.Errorf("build CEL response env: %w", err) } - return &Env{env: e, cache: map[string]cel.Program{}}, nil + + return &Env{ + requestEnv: reqEnv, + responseEnv: respEnv, + cache: map[cacheKey]cel.Program{}, + }, nil } -// Compile turns a CEL source string into a runnable Program. Programs are -// cached by source so repeated compilations of the same expression are free. -// The CEL program must return a boolean - non-boolean results are treated as -// an evaluation error at runtime. -func (e *Env) Compile(src string) (cel.Program, error) { +func (e *Env) envForScope(scope Scope) (*cel.Env, error) { + switch scope { + case ScopeRequest: + return e.requestEnv, nil + case ScopeResponse: + return e.responseEnv, nil + default: + return nil, fmt.Errorf("unknown scope: %v", scope) + } +} + +func (e *Env) compile(src string, scope Scope, kind string, expectedType *cel.Type) (cel.Program, error) { + key := cacheKey{scope: scope, kind: kind, src: src} e.mu.RLock() - if p, ok := e.cache[src]; ok { + if p, ok := e.cache[key]; ok { e.mu.RUnlock() return p, nil } e.mu.RUnlock() - ast, iss := e.env.Compile(src) + cenv, err := e.envForScope(scope) + if err != nil { + return nil, err + } + + ast, iss := cenv.Compile(src) if iss.Err() != nil { - return nil, fmt.Errorf("compile %q: %w", src, iss.Err()) + return nil, fmt.Errorf("compile %q in scope %v: %w", src, scope, iss.Err()) } - if t := ast.OutputType(); t != cel.BoolType && t != cel.DynType { - return nil, fmt.Errorf("expression must return bool, got %s for %q", t.String(), src) + if t := ast.OutputType(); t != expectedType && !t.IsExactType(expectedType) && t != cel.DynType { + return nil, fmt.Errorf("expression must return %s, got %s for %q", expectedType.String(), t.String(), src) } - prog, err := e.env.Program(ast, + prog, err := cenv.Program(ast, cel.EvalOptions(cel.OptOptimize), cel.InterruptCheckFrequency(100), ) @@ -87,23 +125,37 @@ func (e *Env) Compile(src string) (cel.Program, error) { } e.mu.Lock() - e.cache[src] = prog + e.cache[key] = prog e.mu.Unlock() return prog, nil } -// Eval runs a previously compiled program with the given `request` and -// `facts` activation values. Both are passed as map for -// flexibility - the keys must match the variables declared by the env. -// Returns the boolean result; any conversion failure surfaces as an error. -func Eval(ctx context.Context, prog cel.Program, requestVar, factsVar map[string]any) (bool, error) { - if factsVar == nil { - factsVar = map[string]any{} - } - out, _, err := prog.ContextEval(ctx, map[string]any{ - "request": requestVar, - "facts": factsVar, - }) +// Compile compiles a CEL source expecting a boolean result. +func (e *Env) Compile(src string, scope Scope) (cel.Program, error) { + return e.compile(src, scope, "bool", cel.BoolType) +} + +// CompileString compiles a CEL source expecting a string result. +func (e *Env) CompileString(src string, scope Scope) (cel.Program, error) { + return e.compile(src, scope, "string", cel.StringType) +} + +// CompileInt compiles a CEL source expecting an integer result. +func (e *Env) CompileInt(src string, scope Scope) (cel.Program, error) { + return e.compile(src, scope, "int", cel.IntType) +} + +// CompileStringMap compiles a CEL source expecting a map[string]string result. +func (e *Env) CompileStringMap(src string, scope Scope) (cel.Program, error) { + return e.compile(src, scope, "stringmap", cel.MapType(cel.StringType, cel.StringType)) +} + +// Eval runs a program with vars, expecting a boolean result. +func Eval(ctx context.Context, prog cel.Program, vars map[string]any) (bool, error) { + if vars == nil { + vars = map[string]any{} + } + out, _, err := prog.ContextEval(ctx, vars) if err != nil { return false, err } @@ -111,12 +163,105 @@ func Eval(ctx context.Context, prog cel.Program, requestVar, factsVar map[string case types.Bool: return bool(v), nil } - // Best-effort conversion: anything truthy/falsy via CEL's own rules. if b, ok := out.Value().(bool); ok { return b, nil } return false, fmt.Errorf("expression did not evaluate to a bool, got %T", out.Value()) } +// EvalString runs a program with vars, expecting a string result. +func EvalString(ctx context.Context, prog cel.Program, vars map[string]any) (string, error) { + if vars == nil { + vars = map[string]any{} + } + out, _, err := prog.ContextEval(ctx, vars) + if err != nil { + return "", err + } + switch v := out.(type) { + case types.String: + return string(v), nil + } + if s, ok := out.Value().(string); ok { + return s, nil + } + return "", fmt.Errorf("expression did not evaluate to a string, got %T", out.Value()) +} + +// EvalInt runs a program with vars, expecting an integer result. +func EvalInt(ctx context.Context, prog cel.Program, vars map[string]any) (int64, error) { + if vars == nil { + vars = map[string]any{} + } + out, _, err := prog.ContextEval(ctx, vars) + if err != nil { + return 0, err + } + switch v := out.(type) { + case types.Int: + return int64(v), nil + } + switch val := out.Value().(type) { + case int64: + return val, nil + case int: + return int64(val), nil + case int32: + return int64(val), nil + } + return 0, fmt.Errorf("expression did not evaluate to an int, got %T", out.Value()) +} + +// EvalStringMap runs a program with vars, expecting a map[string]string result. +func EvalStringMap(ctx context.Context, prog cel.Program, vars map[string]any) (map[string]string, error) { + if vars == nil { + vars = map[string]any{} + } + out, _, err := prog.ContextEval(ctx, vars) + if err != nil { + return nil, err + } + if out == nil { + return nil, fmt.Errorf("expression evaluated to nil") + } + val := out.Value() + if val == nil { + return nil, fmt.Errorf("expression evaluated to a nil value") + } + + rv := reflect.ValueOf(val) + if rv.Kind() != reflect.Map { + return nil, fmt.Errorf("expression did not evaluate to a map, got %T", val) + } + + res := make(map[string]string, rv.Len()) + iter := rv.MapRange() + for iter.Next() { + k := iter.Key().Interface() + v := iter.Value().Interface() + + // Unwrap ref.Val if needed + if refK, ok := k.(ref.Val); ok { + k = refK.Value() + } + if refV, ok := v.(ref.Val); ok { + v = refV.Value() + } + + kStr, ok := k.(string) + if !ok { + return nil, fmt.Errorf("directResponse headers must be map, key is not a string (got %T)", k) + } + + vStr, ok := v.(string) + if !ok { + return nil, fmt.Errorf("directResponse headers must be map, value for key %q is %T", kStr, v) + } + + res[kStr] = vStr + } + return res, nil +} + // boolVal is a tiny helper to return a CEL bool from Go bool. func boolVal(b bool) ref.Val { return types.Bool(b) } diff --git a/internal/celenv/env_test.go b/internal/celenv/env_test.go index c2ec6df..0f9c3f4 100644 --- a/internal/celenv/env_test.go +++ b/internal/celenv/env_test.go @@ -2,28 +2,12 @@ package celenv import ( "context" + "strings" "testing" ) -func evalBool(t *testing.T, env *Env, src string, req map[string]any) bool { - t.Helper() - prog, err := env.Compile(src) - if err != nil { - t.Fatalf("compile %q: %v", src, err) - } - out, err := Eval(context.Background(), prog, req, nil) - if err != nil { - t.Fatalf("eval %q: %v", src, err) - } - return out -} - -func TestEnvSmoke(t *testing.T) { - env, err := New() - if err != nil { - t.Fatal(err) - } - req := map[string]any{ +func sampleRequest() map[string]any { + return map[string]any{ "method": "POST", "host": "auth.example-1.com", "path": "/realms/mcp/clients-registrations", @@ -50,7 +34,56 @@ func TestEnvSmoke(t *testing.T) { "contentType": "application/json", }, } +} +func sampleResponse() map[string]any { + return map[string]any{ + "status": int64(302), + "headers": map[string][]string{ + "location": {"https://attacker.example/cb"}, + }, + "header": map[string]string{ + "location": "https://attacker.example/cb", + }, + "body": map[string]any{ + "raw": "", + "json": map[string]any{}, + "jsonOk": false, + "yaml": map[string]any{}, + "yamlOk": false, + "size": int64(0), + "contentType": "text/html", + }, + } +} + +func requestVars() map[string]any { + return map[string]any{"request": sampleRequest(), "facts": map[string]any{}} +} + +func responseVars() map[string]any { + return map[string]any{"request": sampleRequest(), "response": sampleResponse(), "facts": map[string]any{}} +} + +func evalBool(t *testing.T, env *Env, src string, scope Scope, vars map[string]any) bool { + t.Helper() + prog, err := env.Compile(src, scope) + if err != nil { + t.Fatalf("compile %q: %v", src, err) + } + out, err := Eval(context.Background(), prog, vars) + if err != nil { + t.Fatalf("eval %q: %v", src, err) + } + return out +} + +func TestEnvSmoke_RequestScopeBooleans(t *testing.T) { + env, err := New() + if err != nil { + t.Fatal(err) + } + vars := requestVars() cases := []struct { expr string want bool @@ -79,8 +112,259 @@ func TestEnvSmoke(t *testing.T) { {`now() > timestamp("2000-01-01T00:00:00Z")`, true}, } for _, c := range cases { - if got := evalBool(t, env, c.expr, req); got != c.want { + if got := evalBool(t, env, c.expr, ScopeRequest, vars); got != c.want { t.Errorf("%q => %v, want %v", c.expr, got, c.want) } } } + +// Canary: response must NOT be reachable in the request scope. If someone +// declares response globally instead of per-scope, this compile stops failing. +func TestCompile_RejectsResponseVarInRequestScope(t *testing.T) { + env, err := New() + if err != nil { + t.Fatal(err) + } + if _, err := env.Compile(`response.status == 302`, ScopeRequest); err == nil { + t.Fatal("expected compile error referencing response in request scope, got nil") + } else if !strings.Contains(err.Error(), "response") { + t.Fatalf("error should mention the undeclared response variable, got: %v", err) + } +} + +func TestCompile_AllowsResponseVarInResponseScope(t *testing.T) { + env, err := New() + if err != nil { + t.Fatal(err) + } + if got := evalBool(t, env, `response.status == 302`, ScopeResponse, responseVars()); !got { + t.Fatal("response.status == 302 should be true with the sample response") + } +} + +// Canary: the cache must not let a source compiled in one scope leak into the +// other. response.status is illegal in request scope and legal in response +// scope; both behaviours must hold regardless of compilation order. +func TestCompile_CacheIsScopeAware(t *testing.T) { + env, err := New() + if err != nil { + t.Fatal(err) + } + const src = `response.status == 302` + if _, err := env.Compile(src, ScopeRequest); err == nil { + t.Fatal("request scope must reject response.status") + } + if _, err := env.Compile(src, ScopeResponse); err != nil { + t.Fatalf("response scope must accept response.status, got: %v", err) + } + if _, err := env.Compile(src, ScopeRequest); err == nil { + t.Fatal("request scope must still reject response.status after caching in response scope") + } +} + +func TestCompileString_AcceptsStringRejectsBool(t *testing.T) { + env, err := New() + if err != nil { + t.Fatal(err) + } + prog, err := env.CompileString(`"https://gw/warn?t=" + request.host`, ScopeRequest) + if err != nil { + t.Fatalf("compile string expr: %v", err) + } + got, err := EvalString(context.Background(), prog, requestVars()) + if err != nil { + t.Fatalf("eval string: %v", err) + } + if got != "https://gw/warn?t=auth.example-1.com" { + t.Fatalf("unexpected string result: %q", got) + } + if _, err := env.CompileString(`request.method == "POST"`, ScopeRequest); err == nil { + t.Fatal("CompileString must reject a bool expression") + } +} + +func TestCompileInt_AcceptsIntFromStatus(t *testing.T) { + env, err := New() + if err != nil { + t.Fatal(err) + } + prog, err := env.CompileInt(`response.status`, ScopeResponse) + if err != nil { + t.Fatalf("compile int expr: %v", err) + } + got, err := EvalInt(context.Background(), prog, responseVars()) + if err != nil { + t.Fatalf("eval int: %v", err) + } + if got != 302 { + t.Fatalf("want 302, got %d", got) + } +} + +// A dyn-typed expression that resolves to a non-string at runtime must surface +// an error from EvalString rather than a wrong value. +func TestEvalString_ErrorsOnNonStringRuntimeValue(t *testing.T) { + env, err := New() + if err != nil { + t.Fatal(err) + } + prog, err := env.CompileString(`request.body.json.redirect_uris`, ScopeRequest) + if err != nil { + t.Fatalf("compile dyn expr: %v", err) + } + if _, err := EvalString(context.Background(), prog, requestVars()); err == nil { + t.Fatal("EvalString must error when the runtime value is not a string") + } +} + +func TestCompileStringMap_Literal(t *testing.T) { + env, err := New() + if err != nil { + t.Fatal(err) + } + + expr := `{"content-type": "text/html", "cache-control": "no-store"}` + prog, err := env.CompileStringMap(expr, ScopeRequest) + if err != nil { + t.Fatalf("CompileStringMap failed: %v", err) + } + + got, err := EvalStringMap(context.Background(), prog, requestVars()) + if err != nil { + t.Fatalf("EvalStringMap failed: %v", err) + } + + want := map[string]string{ + "content-type": "text/html", + "cache-control": "no-store", + } + + if len(got) != len(want) { + t.Fatalf("expected map of size %d, got %d: %+v", len(want), len(got), got) + } + for k, v := range want { + if got[k] != v { + t.Errorf("key %q: got %q, want %q", k, got[k], v) + } + } +} + +func TestCompileStringMap_ResponseHeader(t *testing.T) { + env, err := New() + if err != nil { + t.Fatal(err) + } + + expr := `response.header` + prog, err := env.CompileStringMap(expr, ScopeResponse) + if err != nil { + t.Fatalf("CompileStringMap for response.header failed: %v", err) + } + + got, err := EvalStringMap(context.Background(), prog, responseVars()) + if err != nil { + t.Fatalf("EvalStringMap failed: %v", err) + } + + want := "https://attacker.example/cb" + if got["location"] != want { + t.Errorf("expected location header to be %q, got %q (entire map: %+v)", want, got["location"], got) + } +} + +func TestCompileStringMap_RejectAtCompileTime(t *testing.T) { + env, err := New() + if err != nil { + t.Fatal(err) + } + + invalidExprs := []string{ + `"hello"`, + `200`, + } + + for _, expr := range invalidExprs { + _, err := env.CompileStringMap(expr, ScopeResponse) + if err == nil { + t.Errorf("CompileStringMap should have failed at compile-time for %q", expr) + } else if !strings.Contains(err.Error(), "must return map(string, string)") { + t.Errorf("unexpected error message for %q: %v", expr, err) + } + } +} + +func TestEvalStringMap_Canary_RuntimeTypeError(t *testing.T) { + env, err := New() + if err != nil { + t.Fatal(err) + } + + // Testing expressions that are dyn at compile-time, but evaluate to a non-string-map at runtime + invalidExprs := []struct { + expr string + scope Scope + vars map[string]any + }{ + {`request.body.json`, ScopeRequest, requestVars()}, + {`response.headers`, ScopeResponse, responseVars()}, + } + + for _, tc := range invalidExprs { + prog, err := env.CompileStringMap(tc.expr, tc.scope) + if err != nil { + t.Fatalf("CompileStringMap for %q failed: %v", tc.expr, err) + } + + _, err = EvalStringMap(context.Background(), prog, tc.vars) + if err == nil { + t.Fatalf("EvalStringMap should have failed for %q", tc.expr) + } + + // Check if we get the clear requested error format + expectedSubstr := "directResponse headers must be map" + if !strings.Contains(err.Error(), expectedSubstr) { + t.Errorf("expected error to contain %q, got: %v", expectedSubstr, err) + } + } +} + +func TestEvalStringMap_Canary_RuntimeNonMapError(t *testing.T) { + env, err := New() + if err != nil { + t.Fatal(err) + } + + // Compile a dyn expression that resolves to a string + expr := `request.method` + prog, err := env.CompileStringMap(expr, ScopeRequest) + if err != nil { + t.Fatalf("CompileStringMap for %q failed: %v", expr, err) + } + + _, err = EvalStringMap(context.Background(), prog, requestVars()) + if err == nil { + t.Fatal("EvalStringMap should have failed for a non-map runtime value") + } + + expectedSubstr := "expression did not evaluate to a map" + if !strings.Contains(err.Error(), expectedSubstr) { + t.Errorf("expected error to contain %q, got: %v", expectedSubstr, err) + } +} + +func TestCompileStringMap_MapMergeNotSupported(t *testing.T) { + env, err := New() + if err != nil { + t.Fatal(err) + } + + // Try to use '+' operator to merge maps. Let's see if this compiles in CEL. + // CEL does not natively support map merge with '+', so this should fail to compile. + expr := `{"content-type": "text/html"} + {"cache-control": "no-store"}` + _, err = env.CompileStringMap(expr, ScopeRequest) + if err == nil { + t.Fatal("expected compilation to fail for map '+' operator, or does this version of cel-go actually support it?") + } else { + t.Logf("CompileStringMap for map '+' operator failed as expected: %v", err) + } +} diff --git a/internal/celenv/http.go b/internal/celenv/http.go index c13a78c..e05587f 100644 --- a/internal/celenv/http.go +++ b/internal/celenv/http.go @@ -10,20 +10,20 @@ import ( // request.query. These complement the convenience projections built into the // request object: // -// request.header map first value per (lowercase) header name -// request.headers map> every value per (lowercase) header name -// request.query map first value per query parameter -// request.queries map> every value per query parameter +// request.header map first value per (lowercase) header name +// request.headers map> every value per (lowercase) header name +// request.query map first value per query parameter +// request.queries map> every value per query parameter // // With those projections most lookups are just: // -// request.header['x-api-key'] != '' -// request.query['debug'] == '1' +// request.header['x-api-key'] != '' +// request.query['debug'] == '1' // // The following functions add small ergonomic touches: // -// has(name, bucket) bool true if name exists in the given bucket and has a non-empty value -// firstOr(bucket, name, d) string first value of name in bucket, or d if not present +// has(name, bucket) bool true if name exists in the given bucket and has a non-empty value +// firstOr(bucket, name, d) string first value of name in bucket, or d if not present // // They are generic enough to apply to either headers or query buckets. func httpShortcutsLibrary() cel.EnvOption { diff --git a/internal/celenv/strings.go b/internal/celenv/strings.go index 7e12f66..f4c6fc6 100644 --- a/internal/celenv/strings.go +++ b/internal/celenv/strings.go @@ -20,10 +20,10 @@ import ( // // Pattern syntax: // -// * matches any sequence of characters except '/' -// ** matches any sequence of characters INCLUDING '/' -// ? matches exactly one character -// [abc] character class +// - matches any sequence of characters except '/' +// ** matches any sequence of characters INCLUDING '/' +// ? matches exactly one character +// [abc] character class // // The patterns are translated to RE2 internally and compiled once per pattern. func stringLibrary() cel.EnvOption { diff --git a/internal/facts/facts.go b/internal/facts/facts.go index 40857b4..fe8c795 100644 --- a/internal/facts/facts.go +++ b/internal/facts/facts.go @@ -7,18 +7,18 @@ // // Three retrieval methods are supported, distinguished by the `method` field: // -// value: literal value declared inline in the YAML. Any YAML scalar, -// list or map. Exposed to CEL untouched. +// value: literal value declared inline in the YAML. Any YAML scalar, +// list or map. Exposed to CEL untouched. // -// file: bytes read from a file on disk. Re-read on demand by the policy -// watcher (which already invalidates the whole policy on any -// change inside the config directory). Exposed to CEL as a string. -// Parse it in CEL with `parseJSON(...)` / `parseYAML(...)`. +// file: bytes read from a file on disk. Re-read on demand by the policy +// watcher (which already invalidates the whole policy on any +// change inside the config directory). Exposed to CEL as a string. +// Parse it in CEL with `parseJSON(...)` / `parseYAML(...)`. // -// url: bytes fetched periodically by a background goroutine. Refreshed -// every `interval`. The most recently successful body is what -// CEL sees; if the next fetch fails the previous body is kept. -// Exposed to CEL as a string. +// url: bytes fetched periodically by a background goroutine. Refreshed +// every `interval`. The most recently successful body is what +// CEL sees; if the next fetch fails the previous body is kept. +// Exposed to CEL as a string. // // Usage is split between construction and runtime: New(specs) parses the // declarations and pre-populates inline entries without touching the network, @@ -46,11 +46,11 @@ import ( // Spec is the parsed YAML declaration of one facts entry. type Spec struct { - Name string `yaml:"name"` - Method string `yaml:"method"` // "value" | "file" | "url" - Value any `yaml:"value,omitempty"` - File *File `yaml:"file,omitempty"` - URL *URL `yaml:"url,omitempty"` + Name string `yaml:"name"` + Method string `yaml:"method"` // "value" | "file" | "url" + Value any `yaml:"value,omitempty"` + File *File `yaml:"file,omitempty"` + URL *URL `yaml:"url,omitempty"` } // File is the spec for the `file` method. diff --git a/internal/grpcserver/server.go b/internal/grpcserver/server.go new file mode 100644 index 0000000..f7c7362 --- /dev/null +++ b/internal/grpcserver/server.go @@ -0,0 +1,622 @@ +// Package grpcserver implements the Envoy external processing (ext_proc) gRPC server. +package grpcserver + +import ( + "errors" + "fmt" + "io" + "net" + "net/http" + "sort" + "strconv" + "strings" + "sync/atomic" + + corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + epb "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3" + typev3 "github.com/envoyproxy/go-control-plane/envoy/type/v3" + "google.golang.org/grpc" + + "request-validator/internal/log" + "request-validator/internal/policy" +) + +// Server implements the ExternalProcessorServer interface. +type Server struct { + epb.UnimplementedExternalProcessorServer + policy atomic.Pointer[policy.Config] + srv *grpc.Server +} + +// New builds a server bound to the given initial policy. +func New(initial *policy.Config) *Server { + s := &Server{} + s.policy.Store(initial) + return s +} + +// SetPolicy atomically installs a new policy and returns the old one. +func (s *Server) SetPolicy(c *policy.Config) *policy.Config { + return s.policy.Swap(c) +} + +// Policy returns the currently installed policy. +func (s *Server) Policy() *policy.Config { + return s.policy.Load() +} + +// Run starts the TCP listener and blocks until Stop or fatal error. +func (s *Server) Run(addr string) error { + lis, err := net.Listen("tcp", addr) + if err != nil { + return err + } + + s.srv = grpc.NewServer() + epb.RegisterExternalProcessorServer(s.srv, s) + + log.Infow("starting grpc server", "addr", addr) + err = s.srv.Serve(lis) + if errors.Is(err, grpc.ErrServerStopped) { + return nil + } + return err +} + +// Stop gracefully shuts down the server. +func (s *Server) Stop() { + if s.srv != nil { + s.srv.GracefulStop() + } +} + +// Process implements the bidirectional processing stream. +func (s *Server) Process(stream epb.ExternalProcessor_ProcessServer) error { + ctx := stream.Context() + var req *policy.Request + var resp *policy.Response + + for { + reqMsg, err := stream.Recv() + if errors.Is(err, io.EOF) { + return nil + } + if err != nil { + return err + } + + p := s.policy.Load() + if p == nil { + respMsg := buildContinueResponse(reqMsg) + if err := stream.Send(respMsg); err != nil { + return err + } + continue + } + + switch r := reqMsg.Request.(type) { + case *epb.ProcessingRequest_RequestHeaders: + req = parseRequestHeaders(r.RequestHeaders) + res := p.EvaluateProc(ctx, "requestHeaders", req, nil) + respMsg := s.handleProcResult("requestHeaders", res, p) + if err := stream.Send(respMsg); err != nil { + return err + } + + case *epb.ProcessingRequest_RequestBody: + if req == nil { + req = &policy.Request{Headers: make(http.Header)} + } + bodyChunk := r.RequestBody.Body + req.Body = append(req.Body, bodyChunk...) + + limit := p.Defaults.ExtProc.MaxBodyBytes.Int64() + if int64(len(req.Body)) > limit { + onBodyOverflow := p.Defaults.ExtProc.OnBodyOverflow + log.Warnw("ext_proc body overflow", + "phase", "requestBody", + "limit", limit, + "body_size", len(req.Body), + "action", onBodyOverflow, + "dry_run", p.Defaults.DryRun, + ) + if onBodyOverflow == "fail" { + if p.Defaults.DryRun { + respMsg := &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_RequestBody{ + RequestBody: &epb.BodyResponse{ + Response: &epb.CommonResponse{ + Status: epb.CommonResponse_CONTINUE, + }, + }, + }, + } + if err := stream.Send(respMsg); err != nil { + return err + } + } else { + respMsg := &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_ImmediateResponse{ + ImmediateResponse: &epb.ImmediateResponse{ + Status: &typev3.HttpStatus{ + Code: typev3.StatusCode(500), + }, + Details: "ext_proc body overflow", + }, + }, + } + if err := stream.Send(respMsg); err != nil { + return err + } + } + } else { + respMsg := &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_RequestBody{ + RequestBody: &epb.BodyResponse{ + Response: &epb.CommonResponse{ + Status: epb.CommonResponse_CONTINUE, + }, + }, + }, + } + if err := stream.Send(respMsg); err != nil { + return err + } + } + continue + } + + res := p.EvaluateProc(ctx, "requestBody", req, nil) + respMsg := s.handleProcResult("requestBody", res, p) + if err := stream.Send(respMsg); err != nil { + return err + } + + case *epb.ProcessingRequest_ResponseHeaders: + if req == nil { + req = &policy.Request{Headers: make(http.Header)} + } + resp = parseResponseHeaders(r.ResponseHeaders) + res := p.EvaluateProc(ctx, "responseHeaders", req, resp) + respMsg := s.handleProcResult("responseHeaders", res, p) + if err := stream.Send(respMsg); err != nil { + return err + } + + case *epb.ProcessingRequest_ResponseBody: + if req == nil { + req = &policy.Request{Headers: make(http.Header)} + } + if resp == nil { + resp = &policy.Response{Headers: make(http.Header)} + } + bodyChunk := r.ResponseBody.Body + resp.Body = append(resp.Body, bodyChunk...) + + limit := p.Defaults.ExtProc.MaxBodyBytes.Int64() + if int64(len(resp.Body)) > limit { + onBodyOverflow := p.Defaults.ExtProc.OnBodyOverflow + log.Warnw("ext_proc body overflow", + "phase", "responseBody", + "limit", limit, + "body_size", len(resp.Body), + "action", onBodyOverflow, + "dry_run", p.Defaults.DryRun, + ) + if onBodyOverflow == "fail" { + if p.Defaults.DryRun { + respMsg := &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_ResponseBody{ + ResponseBody: &epb.BodyResponse{ + Response: &epb.CommonResponse{ + Status: epb.CommonResponse_CONTINUE, + }, + }, + }, + } + if err := stream.Send(respMsg); err != nil { + return err + } + } else { + respMsg := &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_ImmediateResponse{ + ImmediateResponse: &epb.ImmediateResponse{ + Status: &typev3.HttpStatus{ + Code: typev3.StatusCode(500), + }, + Details: "ext_proc body overflow", + }, + }, + } + if err := stream.Send(respMsg); err != nil { + return err + } + } + } else { + respMsg := &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_ResponseBody{ + ResponseBody: &epb.BodyResponse{ + Response: &epb.CommonResponse{ + Status: epb.CommonResponse_CONTINUE, + }, + }, + }, + } + if err := stream.Send(respMsg); err != nil { + return err + } + } + continue + } + + res := p.EvaluateProc(ctx, "responseBody", req, resp) + respMsg := s.handleProcResult("responseBody", res, p) + if err := stream.Send(respMsg); err != nil { + return err + } + + default: + respMsg := buildContinueResponse(reqMsg) + if err := stream.Send(respMsg); err != nil { + return err + } + } + } +} + +// handleProcResult filters shadow mutations and logs/builds the processing response. +func (s *Server) handleProcResult(phase string, res policy.ProcResult, p *policy.Config) *epb.ProcessingResponse { + dryGlobal := p.Defaults.DryRun + + // 1. CORTOCIRCUITO: check for first applied directResponse + for _, m := range res.Mutations { + effectiveDry := dryGlobal || m.DryRun + if m.Op == "directResponse" && !effectiveDry { + log.Infow("extProc phase evaluated", + "engine", "extProc", + "phase", phase, + "direct_response", fmt.Sprintf("%s:%d", m.Rule, m.RespStatus), + "dry_run", false, + ) + + return &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_ImmediateResponse{ + ImmediateResponse: &epb.ImmediateResponse{ + Status: &typev3.HttpStatus{ + Code: typev3.StatusCode(m.RespStatus), + }, + Headers: buildHeaderMutationFromMap(m.RespHeaders), + Body: []byte(m.RespBody), + }, + }, + } + } + } + + var applied []policy.ResolvedMutation + var appliedLog []string + var shadowLog []string + + for _, m := range res.Mutations { + effectiveDry := dryGlobal || m.DryRun + if effectiveDry { + switch m.Op { + case "setHeader", "appendHeader": + shadowLog = append(shadowLog, fmt.Sprintf("%s:%s(%s=%s)", m.Rule, m.Op, m.Name, m.Value)) + case "removeHeader": + shadowLog = append(shadowLog, fmt.Sprintf("%s:%s(%s)", m.Rule, m.Op, m.Name)) + case "setBody": + shadowLog = append(shadowLog, fmt.Sprintf("%s:%s(len=%d)", m.Rule, m.Op, len(m.Value))) + case "setStatus": + shadowLog = append(shadowLog, fmt.Sprintf("%s:%s(%d)", m.Rule, m.Op, m.Status)) + case "directResponse": + shadowLog = append(shadowLog, fmt.Sprintf("%s:would directRespond(%d)", m.Rule, m.RespStatus)) + } + } else { + if m.Op == "directResponse" { + continue + } + applied = append(applied, m) + switch m.Op { + case "setHeader", "appendHeader": + appliedLog = append(appliedLog, fmt.Sprintf("%s:%s(%s=%s)", m.Rule, m.Op, m.Name, m.Value)) + case "removeHeader": + appliedLog = append(appliedLog, fmt.Sprintf("%s:%s(%s)", m.Rule, m.Op, m.Name)) + case "setBody": + appliedLog = append(appliedLog, fmt.Sprintf("%s:%s(len=%d)", m.Rule, m.Op, len(m.Value))) + case "setStatus": + appliedLog = append(appliedLog, fmt.Sprintf("%s:%s(%d)", m.Rule, m.Op, m.Status)) + } + } + } + + log.Infow("extProc phase evaluated", + "engine", "extProc", + "phase", phase, + "applied", appliedLog, + "shadow", shadowLog, + "dry_run", dryGlobal, + ) + + hm, bm := buildMutations(applied) + common := &epb.CommonResponse{ + Status: epb.CommonResponse_CONTINUE, + HeaderMutation: hm, + BodyMutation: bm, + } + + switch phase { + case "requestHeaders": + return &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_RequestHeaders{ + RequestHeaders: &epb.HeadersResponse{ + Response: common, + }, + }, + } + case "requestBody": + return &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_RequestBody{ + RequestBody: &epb.BodyResponse{ + Response: common, + }, + }, + } + case "responseHeaders": + return &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_ResponseHeaders{ + ResponseHeaders: &epb.HeadersResponse{ + Response: common, + }, + }, + } + case "responseBody": + return &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_ResponseBody{ + ResponseBody: &epb.BodyResponse{ + Response: common, + }, + }, + } + default: + return &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_RequestHeaders{ + RequestHeaders: &epb.HeadersResponse{ + Response: common, + }, + }, + } + } +} + +// buildHeaderMutationFromMap helper builds HeaderMutation from map deterministically. +func buildHeaderMutationFromMap(headers map[string]string) *epb.HeaderMutation { + if len(headers) == 0 { + return nil + } + var setHeaders []*corev3.HeaderValueOption + var keys []string + for k := range headers { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + setHeaders = append(setHeaders, &corev3.HeaderValueOption{ + Header: &corev3.HeaderValue{ + Key: k, + RawValue: []byte(headers[k]), + }, + AppendAction: corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD, + }) + } + return &epb.HeaderMutation{ + SetHeaders: setHeaders, + } +} + +// buildMutations translates a list of resolved mutations to Envoy types. +func buildMutations(mutations []policy.ResolvedMutation) (*epb.HeaderMutation, *epb.BodyMutation) { + var setHeaders []*corev3.HeaderValueOption + var removeHeaders []string + var bodyMutation *epb.BodyMutation + + for _, m := range mutations { + switch m.Op { + case "setHeader": + setHeaders = append(setHeaders, &corev3.HeaderValueOption{ + Header: &corev3.HeaderValue{ + Key: m.Name, + RawValue: []byte(m.Value), + }, + AppendAction: corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD, + }) + case "appendHeader": + setHeaders = append(setHeaders, &corev3.HeaderValueOption{ + Header: &corev3.HeaderValue{ + Key: m.Name, + RawValue: []byte(m.Value), + }, + AppendAction: corev3.HeaderValueOption_APPEND_IF_EXISTS_OR_ADD, + }) + case "removeHeader": + removeHeaders = append(removeHeaders, m.Name) + case "setBody": + bodyBytes := []byte(m.Value) + bodyMutation = &epb.BodyMutation{ + Mutation: &epb.BodyMutation_Body{ + Body: bodyBytes, + }, + } + setHeaders = append(setHeaders, &corev3.HeaderValueOption{ + Header: &corev3.HeaderValue{ + Key: "content-length", + RawValue: []byte(strconv.Itoa(len(bodyBytes))), + }, + AppendAction: corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD, + }) + case "setStatus": + setHeaders = append(setHeaders, &corev3.HeaderValueOption{ + Header: &corev3.HeaderValue{ + Key: ":status", + RawValue: []byte(strconv.Itoa(m.Status)), + }, + AppendAction: corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD, + }) + } + } + + var hm *epb.HeaderMutation + if len(setHeaders) > 0 || len(removeHeaders) > 0 { + hm = &epb.HeaderMutation{ + SetHeaders: setHeaders, + RemoveHeaders: removeHeaders, + } + } + + return hm, bodyMutation +} + +// buildContinueResponse returns an empty CONTINUE response matched to the request phase. +func buildContinueResponse(reqMsg *epb.ProcessingRequest) *epb.ProcessingResponse { + common := &epb.CommonResponse{ + Status: epb.CommonResponse_CONTINUE, + } + + switch reqMsg.Request.(type) { + case *epb.ProcessingRequest_RequestHeaders: + return &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_RequestHeaders{ + RequestHeaders: &epb.HeadersResponse{ + Response: common, + }, + }, + } + case *epb.ProcessingRequest_RequestBody: + return &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_RequestBody{ + RequestBody: &epb.BodyResponse{ + Response: common, + }, + }, + } + case *epb.ProcessingRequest_ResponseHeaders: + return &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_ResponseHeaders{ + ResponseHeaders: &epb.HeadersResponse{ + Response: common, + }, + }, + } + case *epb.ProcessingRequest_ResponseBody: + return &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_ResponseBody{ + ResponseBody: &epb.BodyResponse{ + Response: common, + }, + }, + } + default: + return &epb.ProcessingResponse{ + Response: &epb.ProcessingResponse_RequestHeaders{ + RequestHeaders: &epb.HeadersResponse{ + Response: common, + }, + }, + } + } +} + +func parseRequestHeaders(h *epb.HttpHeaders) *policy.Request { + var method, scheme, authority, pathAndQuery string + reqHeaders := make(http.Header) + if h != nil && h.Headers != nil { + for _, hv := range h.Headers.Headers { + val := hv.Value + if val == "" && len(hv.RawValue) > 0 { + val = string(hv.RawValue) + } + key := strings.ToLower(hv.Key) + switch key { + case ":method": + method = val + case ":scheme": + scheme = val + case ":authority": + authority = val + case ":path": + pathAndQuery = val + default: + reqHeaders.Add(hv.Key, val) + } + } + } + + host := authority + if host == "" { + host = reqHeaders.Get("Host") + } + + var path, rawQuery string + if pathAndQuery != "" { + if idx := strings.IndexByte(pathAndQuery, '?'); idx >= 0 { + path = pathAndQuery[:idx] + rawQuery = pathAndQuery[idx+1:] + } else { + path = pathAndQuery + } + } + + return &policy.Request{ + Method: method, + Scheme: scheme, + Host: host, + Path: path, + RawQuery: rawQuery, + RemoteIP: extractClientIP(reqHeaders), + Headers: reqHeaders, + } +} + +func parseResponseHeaders(h *epb.HttpHeaders) *policy.Response { + var statusStr string + respHeaders := make(http.Header) + if h != nil && h.Headers != nil { + for _, hv := range h.Headers.Headers { + val := hv.Value + if val == "" && len(hv.RawValue) > 0 { + val = string(hv.RawValue) + } + key := strings.ToLower(hv.Key) + if key == ":status" { + statusStr = val + } else { + respHeaders.Add(hv.Key, val) + } + } + } + + status := 200 + if statusStr != "" { + if code, err := strconv.Atoi(statusStr); err == nil { + status = code + } + } + + return &policy.Response{ + Status: status, + Headers: respHeaders, + } +} + +func extractClientIP(h http.Header) string { + if xff := h.Get("X-Forwarded-For"); xff != "" { + if i := strings.IndexByte(xff, ','); i > 0 { + return strings.TrimSpace(xff[:i]) + } + return strings.TrimSpace(xff) + } + if xri := h.Get("X-Real-Ip"); xri != "" { + return strings.TrimSpace(xri) + } + return "" +} diff --git a/internal/grpcserver/server_test.go b/internal/grpcserver/server_test.go new file mode 100644 index 0000000..bfab854 --- /dev/null +++ b/internal/grpcserver/server_test.go @@ -0,0 +1,1249 @@ +package grpcserver + +import ( + "context" + "io" + "strconv" + "strings" + "testing" + + corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + epb "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3" + typev3 "github.com/envoyproxy/go-control-plane/envoy/type/v3" + "google.golang.org/grpc" + + "request-validator/internal/policy" +) + +type fakeStream struct { + grpc.ServerStream + ctx context.Context + incoming []*epb.ProcessingRequest + outgoing []*epb.ProcessingResponse +} + +func (f *fakeStream) Context() context.Context { + if f.ctx != nil { + return f.ctx + } + return context.Background() +} + +func (f *fakeStream) Recv() (*epb.ProcessingRequest, error) { + if len(f.incoming) == 0 { + return nil, io.EOF + } + req := f.incoming[0] + f.incoming = f.incoming[1:] + return req, nil +} + +func (f *fakeStream) Send(resp *epb.ProcessingResponse) error { + f.outgoing = append(f.outgoing, resp) + return nil +} + +func mustLoadConfig(t *testing.T, yamlStr string) *policy.Config { + t.Helper() + c, err := policy.LoadBytes([]byte(yamlStr)) + if err != nil { + t.Fatalf("failed to load policy: %v", err) + } + return c +} + +func makeHeaderMap(headers map[string]string) *corev3.HeaderMap { + var list []*corev3.HeaderValue + for k, v := range headers { + list = append(list, &corev3.HeaderValue{ + Key: k, + Value: v, + }) + } + return &corev3.HeaderMap{Headers: list} +} + +// TestResponseHeadersLocation checks: +// - responseHeaders: a policy that reescribes Location in an 302 produce un ProcessingResponse_ResponseHeaders con SetHeaders conteniendo location con el valor esperado y AppendAction OVERWRITE. CANARIO del caso de uso real (Keycloak redirect). +func TestResponseHeadersLocation(t *testing.T) { + yamlStr := ` +defaults: + extProc: + maxBodyBytes: 1024 + onBodyOverflow: fail +groups: + - name: keycloak-redirect + parameters: + engine: extProc + mode: applyAll + phase: responseHeaders + match: "true" + rules: + - name: rewrite-location + match: "response.status == 302" + mutations: + - op: setHeader + name: Location + value: "'https://new-location.com'" +` + cfg := mustLoadConfig(t, yamlStr) + srv := New(cfg) + + stream := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_ResponseHeaders{ + ResponseHeaders: &epb.HttpHeaders{ + Headers: makeHeaderMap(map[string]string{ + ":status": "302", + "Location": "http://old-location.com", + }), + }, + }, + }, + }, + } + + err := srv.Process(stream) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.outgoing)) + } + + resp := stream.outgoing[0] + rh, ok := resp.Response.(*epb.ProcessingResponse_ResponseHeaders) + if !ok { + t.Fatalf("expected ProcessingResponse_ResponseHeaders, got %T", resp.Response) + } + + hm := rh.ResponseHeaders.Response.HeaderMutation + if hm == nil { + t.Fatalf("expected header mutations to be non-nil") + } + + found := false + for _, h := range hm.SetHeaders { + if strings.ToLower(h.Header.Key) == "location" { + found = true + if string(h.Header.RawValue) != "https://new-location.com" { + t.Errorf("expected Location value to be 'https://new-location.com', got %q", string(h.Header.RawValue)) + } + if h.AppendAction != corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD { + t.Errorf("expected OVERWRITE_IF_EXISTS_OR_ADD, got %v", h.AppendAction) + } + } + } + if !found { + t.Errorf("Location header mutation not found") + } +} + +// TestSetHeaderOverwriteAndAppend checks: +// - setHeader OVERWRITE vs appendHeader APPEND: AppendAction correcto en cada uno. +// - removeHeader: aparece en RemoveHeaders. +func TestSetHeaderOverwriteAndAppend(t *testing.T) { + yamlStr := ` +defaults: + extProc: + maxBodyBytes: 1024 + onBodyOverflow: fail +groups: + - name: test-headers + parameters: + engine: extProc + mode: applyAll + phase: requestHeaders + match: "true" + rules: + - name: modify-headers + match: "true" + mutations: + - op: setHeader + name: X-Overwrite + value: "'overwritten'" + - op: appendHeader + name: X-Append + value: "'appended'" + - op: removeHeader + name: X-To-Remove +` + cfg := mustLoadConfig(t, yamlStr) + srv := New(cfg) + + stream := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_RequestHeaders{ + RequestHeaders: &epb.HttpHeaders{ + Headers: makeHeaderMap(map[string]string{ + ":method": "GET", + ":path": "/", + "X-Overwrite": "old", + "X-Append": "old", + "X-To-Remove": "old", + }), + }, + }, + }, + }, + } + + err := srv.Process(stream) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.outgoing)) + } + + resp := stream.outgoing[0] + rh, ok := resp.Response.(*epb.ProcessingResponse_RequestHeaders) + if !ok { + t.Fatalf("expected ProcessingResponse_RequestHeaders, got %T", resp.Response) + } + + hm := rh.RequestHeaders.Response.HeaderMutation + if hm == nil { + t.Fatalf("expected header mutations to be non-nil") + } + + // Verify overwrite + var foundOverwrite, foundAppend bool + for _, h := range hm.SetHeaders { + if strings.ToLower(h.Header.Key) == "x-overwrite" { + foundOverwrite = true + if string(h.Header.RawValue) != "overwritten" { + t.Errorf("expected 'overwritten', got %q", string(h.Header.RawValue)) + } + if h.AppendAction != corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD { + t.Errorf("expected OVERWRITE_IF_EXISTS_OR_ADD, got %v", h.AppendAction) + } + } + if strings.ToLower(h.Header.Key) == "x-append" { + foundAppend = true + if string(h.Header.RawValue) != "appended" { + t.Errorf("expected 'appended', got %q", string(h.Header.RawValue)) + } + if h.AppendAction != corev3.HeaderValueOption_APPEND_IF_EXISTS_OR_ADD { + t.Errorf("expected APPEND_IF_EXISTS_OR_ADD, got %v", h.AppendAction) + } + } + } + + if !foundOverwrite { + t.Errorf("X-Overwrite mutation not found") + } + if !foundAppend { + t.Errorf("X-Append mutation not found") + } + + // Verify remove + foundRemove := false + for _, r := range hm.RemoveHeaders { + if strings.ToLower(r) == "x-to-remove" { + foundRemove = true + } + } + if !foundRemove { + t.Errorf("X-To-Remove not found in RemoveHeaders") + } +} + +// TestSetBodyAndContentLength checks: +// - setBody: BodyMutation.Body correcto Y content-length recalculado en SetHeaders. +func TestSetBodyAndContentLength(t *testing.T) { + yamlStr := ` +defaults: + extProc: + maxBodyBytes: 1024 + onBodyOverflow: fail +groups: + - name: test-body + parameters: + engine: extProc + mode: applyAll + phase: requestBody + match: "true" + rules: + - name: rewrite-body + match: "true" + mutations: + - op: setBody + value: "'new-body-content'" +` + cfg := mustLoadConfig(t, yamlStr) + srv := New(cfg) + + stream := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_RequestBody{ + RequestBody: &epb.HttpBody{ + Body: []byte("old-body"), + }, + }, + }, + }, + } + + err := srv.Process(stream) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.outgoing)) + } + + resp := stream.outgoing[0] + rb, ok := resp.Response.(*epb.ProcessingResponse_RequestBody) + if !ok { + t.Fatalf("expected ProcessingResponse_RequestBody, got %T", resp.Response) + } + + bm := rb.RequestBody.Response.BodyMutation + if bm == nil { + t.Fatalf("expected body mutation to be non-nil") + } + + bodyText := bm.GetBody() + if string(bodyText) != "new-body-content" { + t.Errorf("expected body 'new-body-content', got %q", string(bodyText)) + } + + hm := rb.RequestBody.Response.HeaderMutation + if hm == nil { + t.Fatalf("expected header mutations to be non-nil for content-length recalculation") + } + + foundCL := false + for _, h := range hm.SetHeaders { + if strings.ToLower(h.Header.Key) == "content-length" { + foundCL = true + expectedLen := strconv.Itoa(len("new-body-content")) + if string(h.Header.RawValue) != expectedLen { + t.Errorf("expected content-length %q, got %q", expectedLen, string(h.Header.RawValue)) + } + if h.AppendAction != corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD { + t.Errorf("expected OVERWRITE_IF_EXISTS_OR_ADD, got %v", h.AppendAction) + } + } + } + + if !foundCL { + t.Errorf("content-length header mutation not found") + } +} + +// TestSetStatus checks: +// - setStatus: :status en SetHeaders con el codigo. +func TestSetStatus(t *testing.T) { + yamlStr := ` +defaults: + extProc: + maxBodyBytes: 1024 + onBodyOverflow: fail +groups: + - name: test-status + parameters: + engine: extProc + mode: applyAll + phase: responseHeaders + match: "true" + rules: + - name: rewrite-status + match: "true" + mutations: + - op: setStatus + code: "418" +` + cfg := mustLoadConfig(t, yamlStr) + srv := New(cfg) + + stream := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_ResponseHeaders{ + ResponseHeaders: &epb.HttpHeaders{ + Headers: makeHeaderMap(map[string]string{ + ":status": "200", + }), + }, + }, + }, + }, + } + + err := srv.Process(stream) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.outgoing)) + } + + resp := stream.outgoing[0] + rh, ok := resp.Response.(*epb.ProcessingResponse_ResponseHeaders) + if !ok { + t.Fatalf("expected ProcessingResponse_ResponseHeaders, got %T", resp.Response) + } + + hm := rh.ResponseHeaders.Response.HeaderMutation + if hm == nil { + t.Fatalf("expected header mutations to be non-nil") + } + + foundStatus := false + for _, h := range hm.SetHeaders { + if h.Header.Key == ":status" { + foundStatus = true + if string(h.Header.RawValue) != "418" { + t.Errorf("expected status '418', got %q", string(h.Header.RawValue)) + } + if h.AppendAction != corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD { + t.Errorf("expected OVERWRITE, got %v", h.AppendAction) + } + } + } + + if !foundStatus { + t.Errorf(":status mutation not found") + } +} + +// TestDryRunGlobal checks: +// - dry-run global: una policy con mutaciones produce CONTINUE SIN mutaciones (canario: debe fallar si se aplican bajo dry-run). +func TestDryRunGlobal(t *testing.T) { + yamlStr := ` +defaults: + dryRun: true + extProc: + maxBodyBytes: 1024 + onBodyOverflow: fail +groups: + - name: test-dry-run-global + parameters: + engine: extProc + mode: applyAll + phase: requestHeaders + match: "true" + rules: + - name: dry-run-rule + match: "true" + mutations: + - op: setHeader + name: X-Dry + value: "'not-applied'" +` + cfg := mustLoadConfig(t, yamlStr) + srv := New(cfg) + + stream := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_RequestHeaders{ + RequestHeaders: &epb.HttpHeaders{ + Headers: makeHeaderMap(map[string]string{ + ":method": "GET", + ":path": "/", + }), + }, + }, + }, + }, + } + + err := srv.Process(stream) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.outgoing)) + } + + resp := stream.outgoing[0] + rh, ok := resp.Response.(*epb.ProcessingResponse_RequestHeaders) + if !ok { + t.Fatalf("expected ProcessingResponse_RequestHeaders, got %T", resp.Response) + } + + hm := rh.RequestHeaders.Response.HeaderMutation + if hm != nil { + t.Errorf("expected header mutations to be nil under global dry-run, got %v", hm) + } +} + +// TestDryRunPerRule checks: +// - dry-run por regla: una mutacion de regla dryRun no se aplica, otra normal si. +func TestDryRunPerRule(t *testing.T) { + yamlStr := ` +defaults: + extProc: + maxBodyBytes: 1024 + onBodyOverflow: fail +groups: + - name: test-dry-run-rule + parameters: + engine: extProc + mode: applyAll + phase: requestHeaders + match: "true" + rules: + - name: normal-rule + match: "true" + mutations: + - op: setHeader + name: X-Normal + value: "'yes'" + - name: dry-rule + match: "true" + dryRun: true + mutations: + - op: setHeader + name: X-Dry + value: "'no'" +` + cfg := mustLoadConfig(t, yamlStr) + srv := New(cfg) + + stream := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_RequestHeaders{ + RequestHeaders: &epb.HttpHeaders{ + Headers: makeHeaderMap(map[string]string{ + ":method": "GET", + ":path": "/", + }), + }, + }, + }, + }, + } + + err := srv.Process(stream) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.outgoing)) + } + + resp := stream.outgoing[0] + rh, ok := resp.Response.(*epb.ProcessingResponse_RequestHeaders) + if !ok { + t.Fatalf("expected ProcessingResponse_RequestHeaders, got %T", resp.Response) + } + + hm := rh.RequestHeaders.Response.HeaderMutation + if hm == nil { + t.Fatalf("expected header mutations to be non-nil") + } + + for _, h := range hm.SetHeaders { + if strings.ToLower(h.Header.Key) == "x-dry" { + t.Errorf("X-Dry should NOT be present (dryRun rule)") + } + } + + foundNormal := false + for _, h := range hm.SetHeaders { + if strings.ToLower(h.Header.Key) == "x-normal" { + foundNormal = true + if string(h.Header.RawValue) != "yes" { + t.Errorf("expected 'yes', got %q", string(h.Header.RawValue)) + } + } + } + + if !foundNormal { + t.Errorf("X-Normal should be present") + } +} + +// TestOverflowSkip checks: +// - overflow body skip: body mayor que MaxBodyBytes -> CONTINUE sin mutaciones. +func TestOverflowSkip(t *testing.T) { + yamlStr := ` +defaults: + extProc: + maxBodyBytes: 10 + onBodyOverflow: skip +groups: + - name: test-body-overflow + parameters: + engine: extProc + mode: applyAll + phase: requestBody + match: "true" + rules: + - name: rewrite-body + match: "true" + mutations: + - op: setBody + value: "'new-body'" +` + cfg := mustLoadConfig(t, yamlStr) + srv := New(cfg) + + stream := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_RequestBody{ + RequestBody: &epb.HttpBody{ + Body: []byte("this is more than 10 bytes!"), + }, + }, + }, + }, + } + + err := srv.Process(stream) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.outgoing)) + } + + resp := stream.outgoing[0] + rb, ok := resp.Response.(*epb.ProcessingResponse_RequestBody) + if !ok { + t.Fatalf("expected ProcessingResponse_RequestBody, got %T", resp.Response) + } + + if rb.RequestBody.Response.BodyMutation != nil { + t.Errorf("expected body mutation to be nil on overflow skip") + } + if rb.RequestBody.Response.HeaderMutation != nil { + t.Errorf("expected header mutation to be nil on overflow skip") + } +} + +// TestOverflowFail checks: +// - overflow body fail: -> ImmediateResponse 500. Y bajo dry-run global -> CONTINUE (canario). +func TestOverflowFail(t *testing.T) { + yamlStr := ` +defaults: + extProc: + maxBodyBytes: 10 + onBodyOverflow: fail +groups: + - name: test-body-overflow + parameters: + engine: extProc + mode: applyAll + phase: requestBody + match: "true" + rules: + - name: rewrite-body + match: "true" + mutations: + - op: setBody + value: "'new-body'" +` + // 1. Without dry-run: should fail with ImmediateResponse 500 + cfg1 := mustLoadConfig(t, yamlStr) + srv1 := New(cfg1) + + stream1 := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_RequestBody{ + RequestBody: &epb.HttpBody{ + Body: []byte("this is more than 10 bytes!"), + }, + }, + }, + }, + } + + err := srv1.Process(stream1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream1.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream1.outgoing)) + } + + resp1 := stream1.outgoing[0] + ir, ok := resp1.Response.(*epb.ProcessingResponse_ImmediateResponse) + if !ok { + t.Fatalf("expected ProcessingResponse_ImmediateResponse, got %T", resp1.Response) + } + + if ir.ImmediateResponse.Status.Code != typev3.StatusCode(500) { + t.Errorf("expected status code 500, got %v", ir.ImmediateResponse.Status.Code) + } + if ir.ImmediateResponse.Details != "ext_proc body overflow" { + t.Errorf("expected details 'ext_proc body overflow', got %q", ir.ImmediateResponse.Details) + } + + // 2. With global dry-run: should CONTINUE with no mutations (canario) + yamlStrDry := ` +defaults: + dryRun: true + extProc: + maxBodyBytes: 10 + onBodyOverflow: fail +groups: + - name: test-body-overflow + parameters: + engine: extProc + mode: applyAll + phase: requestBody + match: "true" + rules: + - name: rewrite-body + match: "true" + mutations: + - op: setBody + value: "'new-body'" +` + cfg2 := mustLoadConfig(t, yamlStrDry) + srv2 := New(cfg2) + + stream2 := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_RequestBody{ + RequestBody: &epb.HttpBody{ + Body: []byte("this is more than 10 bytes!"), + }, + }, + }, + }, + } + + err = srv2.Process(stream2) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream2.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream2.outgoing)) + } + + resp2 := stream2.outgoing[0] + rb, ok := resp2.Response.(*epb.ProcessingResponse_RequestBody) + if !ok { + t.Fatalf("expected ProcessingResponse_RequestBody, got %T", resp2.Response) + } + + if rb.RequestBody.Response.BodyMutation != nil { + t.Errorf("expected body mutation to be nil under global dry run") + } +} + +// TestPseudoHeadersParsing checks: +// - parseo de pseudo-headers: ":method"/":path"/":authority"/":status" se reflejan en el policy.Request/Response que ve la policy (verificable via una policy cuyo match use request.method/path o response.status y mute en consecuencia). +func TestPseudoHeadersParsing(t *testing.T) { + yamlStr := ` +defaults: + extProc: + maxBodyBytes: 1024 + onBodyOverflow: fail +groups: + - name: check-pseudo-headers + parameters: + engine: extProc + mode: applyAll + phase: requestHeaders + match: "true" + rules: + - name: match-path + match: "request.path == '/secure' && request.method == 'POST' && request.host == 'example.com' && request.query['a'] == 'b'" + mutations: + - op: setHeader + name: X-Matched-Pseudo + value: "'yes'" +` + cfg := mustLoadConfig(t, yamlStr) + srv := New(cfg) + + stream := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_RequestHeaders{ + RequestHeaders: &epb.HttpHeaders{ + Headers: makeHeaderMap(map[string]string{ + ":method": "POST", + ":path": "/secure?a=b", + ":authority": "example.com", + }), + }, + }, + }, + }, + } + + err := srv.Process(stream) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.outgoing)) + } + + resp := stream.outgoing[0] + rh, ok := resp.Response.(*epb.ProcessingResponse_RequestHeaders) + if !ok { + t.Fatalf("expected ProcessingResponse_RequestHeaders, got %T", resp.Response) + } + + hm := rh.RequestHeaders.Response.HeaderMutation + if hm == nil { + t.Fatalf("expected header mutations to be non-nil") + } + + foundMatched := false + for _, h := range hm.SetHeaders { + if strings.ToLower(h.Header.Key) == "x-matched-pseudo" { + foundMatched = true + if string(h.Header.RawValue) != "yes" { + t.Errorf("expected 'yes', got %q", string(h.Header.RawValue)) + } + } + } + + if !foundMatched { + t.Errorf("X-Matched-Pseudo was not set (pseudo-headers might not have been correctly parsed/used)") + } +} + +// TestNoMatchingGroups checks: +// - fase sin grupos aplicables -> CONTINUE vacio. +func TestNoMatchingGroups(t *testing.T) { + yamlStr := ` +defaults: + extProc: + maxBodyBytes: 1024 + onBodyOverflow: fail +groups: + - name: test-headers + parameters: + engine: extProc + mode: applyAll + phase: requestHeaders + match: "true" + rules: + - name: modify-headers + match: "request.path == '/something-else'" + mutations: + - op: setHeader + name: X-Overwrite + value: "'overwritten'" +` + cfg := mustLoadConfig(t, yamlStr) + srv := New(cfg) + + stream := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_RequestHeaders{ + RequestHeaders: &epb.HttpHeaders{ + Headers: makeHeaderMap(map[string]string{ + ":method": "GET", + ":path": "/", + }), + }, + }, + }, + }, + } + + err := srv.Process(stream) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.outgoing)) + } + + resp := stream.outgoing[0] + rh, ok := resp.Response.(*epb.ProcessingResponse_RequestHeaders) + if !ok { + t.Fatalf("expected ProcessingResponse_RequestHeaders, got %T", resp.Response) + } + + if rh.RequestHeaders.Response.HeaderMutation != nil { + t.Errorf("expected nil HeaderMutation when rules do not match") + } +} + +// TestDirectResponseInResponseHeaders checks: +// - directResponse in responseHeaders: una policy que ante un 302 hace directResponse(200, {content-type: text/html}, body "hola") +// produce un ProcessingResponse_ImmediateResponse con Status.Code==200, Body=="hola" y un header content-type=text/html. +func TestDirectResponseInResponseHeaders(t *testing.T) { + yamlStr := ` +defaults: + extProc: + maxBodyBytes: 1024 + onBodyOverflow: fail +groups: + - name: direct-resp-group + parameters: + engine: extProc + mode: applyAll + phase: responseHeaders + match: "true" + rules: + - name: direct-resp-rule + match: "response.status == 302" + mutations: + - op: directResponse + status: 200 + headers: '{"content-type": "text/html"}' + body: "'hola'" +` + cfg := mustLoadConfig(t, yamlStr) + srv := New(cfg) + + stream := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_ResponseHeaders{ + ResponseHeaders: &epb.HttpHeaders{ + Headers: makeHeaderMap(map[string]string{ + ":status": "302", + }), + }, + }, + }, + }, + } + + err := srv.Process(stream) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.outgoing)) + } + + resp := stream.outgoing[0] + ir, ok := resp.Response.(*epb.ProcessingResponse_ImmediateResponse) + if !ok { + t.Fatalf("expected ProcessingResponse_ImmediateResponse, got %T", resp.Response) + } + + if ir.ImmediateResponse.Status.Code != typev3.StatusCode(200) { + t.Errorf("expected status 200, got %v", ir.ImmediateResponse.Status.Code) + } + + if string(ir.ImmediateResponse.Body) != "hola" { + t.Errorf("expected body 'hola', got %q", string(ir.ImmediateResponse.Body)) + } + + hm := ir.ImmediateResponse.Headers + if hm == nil { + t.Fatalf("expected headers to be non-nil") + } + + found := false + for _, h := range hm.SetHeaders { + if strings.ToLower(h.Header.Key) == "content-type" { + found = true + if string(h.Header.RawValue) != "text/html" { + t.Errorf("expected content-type to be 'text/html', got %q", string(h.Header.RawValue)) + } + if h.AppendAction != corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD { + t.Errorf("expected OVERWRITE_IF_EXISTS_OR_ADD, got %v", h.AppendAction) + } + } + } + if !found { + t.Errorf("content-type header not found in immediate response") + } +} + +// TestDirectResponseCortocircuito checks: +// - cortocircuito: una regla applyAll/firstMatch donde ademas del directResponse hubiera (en otra regla del mismo grupo en applyAll) +// un setHeader: el resultado es ImmediateResponse y el setHeader NO aparece (no es CommonResponse). +func TestDirectResponseCortocircuito(t *testing.T) { + yamlStr := ` +defaults: + extProc: + maxBodyBytes: 1024 + onBodyOverflow: fail +groups: + - name: direct-resp-group + parameters: + engine: extProc + mode: applyAll + phase: responseHeaders + match: "true" + rules: + - name: direct-resp-rule + match: "response.status == 302" + mutations: + - op: directResponse + status: 200 + headers: '{"content-type": "text/html"}' + body: "'hola'" + - name: set-header-rule + match: "true" + mutations: + - op: setHeader + name: X-Should-Not-Exist + value: "'somevalue'" +` + cfg := mustLoadConfig(t, yamlStr) + srv := New(cfg) + + stream := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_ResponseHeaders{ + ResponseHeaders: &epb.HttpHeaders{ + Headers: makeHeaderMap(map[string]string{ + ":status": "302", + }), + }, + }, + }, + }, + } + + err := srv.Process(stream) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.outgoing)) + } + + resp := stream.outgoing[0] + ir, ok := resp.Response.(*epb.ProcessingResponse_ImmediateResponse) + if !ok { + t.Fatalf("expected ProcessingResponse_ImmediateResponse, got %T", resp.Response) + } + + if ir.ImmediateResponse.Headers != nil { + for _, h := range ir.ImmediateResponse.Headers.SetHeaders { + if strings.ToLower(h.Header.Key) == "x-should-not-exist" { + t.Errorf("found X-Should-Not-Exist header, but directResponse should have ignored all other mutations") + } + } + } +} + +// TestDirectResponseDryRunGlobal checks: +// - dry-run global: misma policy con defaults.dryRun: true -> NO ImmediateResponse; debe ser CONTINUE (CommonResponse) sin mutaciones. +func TestDirectResponseDryRunGlobal(t *testing.T) { + yamlStr := ` +defaults: + dryRun: true + extProc: + maxBodyBytes: 1024 + onBodyOverflow: fail +groups: + - name: direct-resp-group + parameters: + engine: extProc + mode: applyAll + phase: responseHeaders + match: "true" + rules: + - name: direct-resp-rule + match: "response.status == 302" + mutations: + - op: directResponse + status: 200 + headers: '{"content-type": "text/html"}' + body: "'hola'" +` + cfg := mustLoadConfig(t, yamlStr) + srv := New(cfg) + + stream := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_ResponseHeaders{ + ResponseHeaders: &epb.HttpHeaders{ + Headers: makeHeaderMap(map[string]string{ + ":status": "302", + }), + }, + }, + }, + }, + } + + err := srv.Process(stream) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.outgoing)) + } + + resp := stream.outgoing[0] + rh, ok := resp.Response.(*epb.ProcessingResponse_ResponseHeaders) + if !ok { + t.Fatalf("expected ProcessingResponse_ResponseHeaders, got %T", resp.Response) + } + + if rh.ResponseHeaders.Response.Status != epb.CommonResponse_CONTINUE { + t.Errorf("expected Status CONTINUE, got %v", rh.ResponseHeaders.Response.Status) + } + + if rh.ResponseHeaders.Response.HeaderMutation != nil { + t.Errorf("expected nil HeaderMutation, got %v", rh.ResponseHeaders.Response.HeaderMutation) + } +} + +// TestDirectResponseDryRunPorRegla checks: +// - dry-run por regla: la regla del directResponse con dryRun: true -> no se sirve (CONTINUE). +func TestDirectResponseDryRunPorRegla(t *testing.T) { + yamlStr := ` +defaults: + extProc: + maxBodyBytes: 1024 + onBodyOverflow: fail +groups: + - name: direct-resp-group + parameters: + engine: extProc + mode: applyAll + phase: responseHeaders + match: "true" + rules: + - name: direct-resp-rule + dryRun: true + match: "response.status == 302" + mutations: + - op: directResponse + status: 200 + headers: '{"content-type": "text/html"}' + body: "'hola'" +` + cfg := mustLoadConfig(t, yamlStr) + srv := New(cfg) + + stream := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_ResponseHeaders{ + ResponseHeaders: &epb.HttpHeaders{ + Headers: makeHeaderMap(map[string]string{ + ":status": "302", + }), + }, + }, + }, + }, + } + + err := srv.Process(stream) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.outgoing)) + } + + resp := stream.outgoing[0] + rh, ok := resp.Response.(*epb.ProcessingResponse_ResponseHeaders) + if !ok { + t.Fatalf("expected ProcessingResponse_ResponseHeaders, got %T", resp.Response) + } + + if rh.ResponseHeaders.Response.Status != epb.CommonResponse_CONTINUE { + t.Errorf("expected Status CONTINUE, got %v", rh.ResponseHeaders.Response.Status) + } + + if rh.ResponseHeaders.Response.HeaderMutation != nil { + t.Errorf("expected nil HeaderMutation, got %v", rh.ResponseHeaders.Response.HeaderMutation) + } +} + +// TestDirectResponseSinDirectResponse checks: +// - sin directResponse: una policy solo con setHeader sigue produciendo CommonResponse con el header (no se rompio el flujo viejo). +func TestDirectResponseSinDirectResponse(t *testing.T) { + yamlStr := ` +defaults: + extProc: + maxBodyBytes: 1024 + onBodyOverflow: fail +groups: + - name: keycloak-redirect + parameters: + engine: extProc + mode: applyAll + phase: responseHeaders + match: "true" + rules: + - name: rewrite-location + match: "response.status == 302" + mutations: + - op: setHeader + name: Location + value: "'https://new-location.com'" +` + cfg := mustLoadConfig(t, yamlStr) + srv := New(cfg) + + stream := &fakeStream{ + incoming: []*epb.ProcessingRequest{ + { + Request: &epb.ProcessingRequest_ResponseHeaders{ + ResponseHeaders: &epb.HttpHeaders{ + Headers: makeHeaderMap(map[string]string{ + ":status": "302", + "Location": "http://old-location.com", + }), + }, + }, + }, + }, + } + + err := srv.Process(stream) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(stream.outgoing) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.outgoing)) + } + + resp := stream.outgoing[0] + rh, ok := resp.Response.(*epb.ProcessingResponse_ResponseHeaders) + if !ok { + t.Fatalf("expected ProcessingResponse_ResponseHeaders, got %T", resp.Response) + } + + hm := rh.ResponseHeaders.Response.HeaderMutation + if hm == nil { + t.Fatalf("expected header mutations to be non-nil") + } + + found := false + for _, h := range hm.SetHeaders { + if strings.ToLower(h.Header.Key) == "location" { + found = true + if string(h.Header.RawValue) != "https://new-location.com" { + t.Errorf("expected Location value to be 'https://new-location.com', got %q", string(h.Header.RawValue)) + } + } + } + if !found { + t.Errorf("Location header mutation not found") + } +} diff --git a/internal/httpserver/metrics.go b/internal/httpserver/metrics.go index ce2abfd..66498cc 100644 --- a/internal/httpserver/metrics.go +++ b/internal/httpserver/metrics.go @@ -15,7 +15,7 @@ type metrics struct { totalDenied atomic.Uint64 totalErrors atomic.Uint64 - mu sync.RWMutex + mu sync.RWMutex perRule map[ruleKey]*ruleCounters } diff --git a/internal/httpserver/server.go b/internal/httpserver/server.go index 3b7ef19..2dc501d 100644 --- a/internal/httpserver/server.go +++ b/internal/httpserver/server.go @@ -114,7 +114,8 @@ func (s *Server) handle(w http.ResponseWriter, r *http.Request) { return } - body, err := io.ReadAll(io.LimitReader(r.Body, p.Defaults.MaxBodyBytes.Int64())) + limit := p.Defaults.ExtAuthz.MaxBodyBytes.Int64() + body, err := io.ReadAll(io.LimitReader(r.Body, limit+1)) if err != nil { // Reading the body failed: the policy cannot be evaluated, so the // default is fail-closed. Under global dry-run the request still passes @@ -128,8 +129,54 @@ func (s *Server) handle(w http.ResponseWriter, r *http.Request) { return } w.Header().Set(hdrDry, "false") - w.WriteHeader(p.Defaults.DenyStatus) - _, _ = w.Write([]byte(p.Defaults.DenyBody)) + w.WriteHeader(p.Defaults.ExtAuthz.DenyStatus) + _, _ = w.Write([]byte(p.Defaults.ExtAuthz.DenyBody)) + return + } + + if int64(len(body)) > limit { + // Overflow: request body exceeded the maximum configured limit. + truncatedBody := body[:limit] + req := &policy.Request{ + Method: r.Method, + Scheme: firstNonEmpty(r.Header.Get("X-Forwarded-Proto"), r.URL.Scheme, "http"), + Host: hostOnly(r.Host), + Path: r.URL.Path, + RawQuery: r.URL.RawQuery, + RemoteIP: clientIP(r), + Headers: r.Header, + Body: truncatedBody, + } + + dry := p.Defaults.DryRun + w.Header().Set(hdrRule, "") + w.Header().Set(hdrReason, "request body too large") + w.Header().Set(hdrResult, "deny") + if dry { + w.Header().Set(hdrDry, "true") + } else { + w.Header().Set(hdrDry, "false") + } + + s.metrics.record("", "deny", dry) + + if dry { + w.WriteHeader(http.StatusOK) + } else { + w.WriteHeader(p.Defaults.ExtAuthz.DenyStatus) + _, _ = w.Write([]byte(p.Defaults.ExtAuthz.DenyBody)) + } + + logger := log.Logger() + rec := []any{ + "decision", "deny", + "rule", "", + "reason", "request body too large", + "dry_run", dry, + "duration_ms", float64(time.Since(start).Microseconds()) / 1000.0, + accessLogAttrs(req, p.Logging), + } + logger.Warn("request decided", rec...) return } @@ -172,8 +219,8 @@ func (s *Server) handle(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) } else { // Real deny, not shadowed: Envoy returns the configured error. - w.WriteHeader(p.Defaults.DenyStatus) - _, _ = w.Write([]byte(p.Defaults.DenyBody)) + w.WriteHeader(p.Defaults.ExtAuthz.DenyStatus) + _, _ = w.Write([]byte(p.Defaults.ExtAuthz.DenyBody)) } // One access-log record per request. Log level mirrors the verdict: diff --git a/internal/httpserver/server_test.go b/internal/httpserver/server_test.go index 80fa2bc..2954c0e 100644 --- a/internal/httpserver/server_test.go +++ b/internal/httpserver/server_test.go @@ -22,14 +22,20 @@ func loadPolicy(t *testing.T, src string) *policy.Config { func TestServerAllowsAndDenies(t *testing.T) { cfg := loadPolicy(t, ` -defaults: { action: deny } +defaults: + extAuthz: + action: deny groups: - name: allow-foo - action: allow + parameters: + engine: extAuthz + mode: firstMatch rules: - name: get-foo match: | request.method == 'GET' && request.path.startsWith('/foo') + validation: + action: allow `) s := New(cfg) ts := httptest.NewServer(http.HandlerFunc(s.handle)) @@ -51,10 +57,14 @@ groups: func TestServerDCREndToEnd(t *testing.T) { cfg := loadPolicy(t, ` -defaults: { action: deny } +defaults: + extAuthz: + action: deny groups: - name: dcr - action: allow + parameters: + engine: extAuthz + mode: firstMatch match: | request.method == 'POST' && request.host == 'auth.example-1.com' rules: @@ -62,6 +72,8 @@ groups: match: | request.body.jsonOk && request.body.json.redirect_uris.all(u, u.startsWith('https://antigravity.google/')) + validation: + action: allow `) s := New(cfg) ts := httptest.NewServer(http.HandlerFunc(s.handle)) @@ -91,12 +103,19 @@ groups: func TestHotReloadSwap(t *testing.T) { initial := loadPolicy(t, ` -defaults: { action: deny } +defaults: + extAuthz: + action: deny groups: - name: noop + parameters: + engine: extAuthz + mode: firstMatch rules: - name: never match: "false" + validation: + action: allow `) s := New(initial) ts := httptest.NewServer(http.HandlerFunc(s.handle)) @@ -108,12 +127,19 @@ groups: } s.SetPolicy(loadPolicy(t, ` -defaults: { action: allow } +defaults: + extAuthz: + action: allow groups: - name: noop + parameters: + engine: extAuthz + mode: firstMatch rules: - name: never match: "false" + validation: + action: allow `)) res, _ = http.Get(ts.URL + "/x") if res.StatusCode != 200 { @@ -121,8 +147,6 @@ groups: } } -// TestServerDryRun covers the per-rule and global dry-run combinations, -// including a mixed group where one rule shadows and another enforces. func TestServerDryRun(t *testing.T) { cases := []struct { name string @@ -133,18 +157,23 @@ func TestServerDryRun(t *testing.T) { wantResult string wantDryRun string }{ - // --- per-rule dryRun --- { name: "per-rule dryRun: deny rule matches yields 200 shadow deny", policyYAML: ` -defaults: { action: allow } +defaults: + extAuthz: + action: allow groups: - name: shadow - action: deny + parameters: + engine: extAuthz + mode: firstMatch rules: - name: would-deny dryRun: true match: "request.path.startsWith('/forbidden')" + validation: + action: deny `, method: "GET", path: "/forbidden/x", @@ -155,14 +184,20 @@ groups: { name: "per-rule dryRun: deny rule does not match, allow normally", policyYAML: ` -defaults: { action: allow } +defaults: + extAuthz: + action: allow groups: - name: shadow - action: deny + parameters: + engine: extAuthz + mode: firstMatch rules: - name: would-deny dryRun: true match: "request.path.startsWith('/forbidden')" + validation: + action: deny `, method: "GET", path: "/safe/path", @@ -170,19 +205,23 @@ groups: wantResult: "allow", wantDryRun: "false", }, - // --- global defaults.dryRun --- { name: "global dryRun: would-deny rule yields 200 shadow deny", policyYAML: ` defaults: - action: deny + extAuthz: + action: deny dryRun: true groups: - name: deny-all - action: deny + parameters: + engine: extAuthz + mode: firstMatch rules: - name: block match: "true" + validation: + action: deny `, method: "GET", path: "/anything", @@ -194,14 +233,19 @@ groups: name: "global dryRun: allow rule yields 200 allow, dry-run flag still true", policyYAML: ` defaults: - action: allow + extAuthz: + action: allow dryRun: true groups: - name: allow-all - action: allow + parameters: + engine: extAuthz + mode: firstMatch rules: - name: pass match: "true" + validation: + action: allow `, method: "GET", path: "/anything", @@ -209,17 +253,22 @@ groups: wantResult: "allow", wantDryRun: "true", }, - // --- regression: dryRun=false (default) behaves as before --- { name: "no dryRun: real deny is enforced, 403", policyYAML: ` -defaults: { action: deny } +defaults: + extAuthz: + action: deny groups: - name: deny-all - action: deny + parameters: + engine: extAuthz + mode: firstMatch rules: - name: block match: "true" + validation: + action: deny `, method: "GET", path: "/anything", @@ -230,13 +279,19 @@ groups: { name: "no dryRun: real allow, 200", policyYAML: ` -defaults: { action: allow } +defaults: + extAuthz: + action: allow groups: - name: allow-all - action: allow + parameters: + engine: extAuthz + mode: firstMatch rules: - name: pass match: "true" + validation: + action: allow `, method: "GET", path: "/anything", @@ -244,19 +299,23 @@ groups: wantResult: "allow", wantDryRun: "false", }, - // --- global dryRun overrides a non-dryRun deny rule --- { name: "global dryRun overrides non-dryRun deny rule", policyYAML: ` defaults: - action: allow + extAuthz: + action: allow dryRun: true groups: - name: strict - action: deny + parameters: + engine: extAuthz + mode: firstMatch rules: - name: block-post match: "request.method == 'POST'" + validation: + action: deny `, method: "POST", path: "/api/resource", @@ -264,20 +323,24 @@ groups: wantResult: "deny", wantDryRun: "true", }, - // --- per-rule dryRun + global dryRun: effectiveDry=true --- { name: "per-rule dryRun AND global dryRun: shadow deny", policyYAML: ` defaults: - action: deny + extAuthz: + action: deny dryRun: true groups: - name: shadow - action: deny + parameters: + engine: extAuthz + mode: firstMatch rules: - name: would-deny dryRun: true match: "true" + validation: + action: deny `, method: "GET", path: "/x", @@ -285,20 +348,27 @@ groups: wantResult: "deny", wantDryRun: "true", }, - // --- mixed group, global off: per-rule dryRun is independent --- { name: "mixed group: dryRun rule decides yields 200 shadow deny", policyYAML: ` -defaults: { action: allow } +defaults: + extAuthz: + action: allow groups: - name: mixed - action: deny + parameters: + engine: extAuthz + mode: firstMatch rules: - name: shadow-admin dryRun: true match: "request.path.startsWith('/admin')" + validation: + action: deny - name: block-internal match: "request.path.startsWith('/internal')" + validation: + action: deny `, method: "GET", path: "/admin/panel", @@ -309,16 +379,24 @@ groups: { name: "mixed group: non-dryRun rule decides yields enforced 403", policyYAML: ` -defaults: { action: allow } +defaults: + extAuthz: + action: allow groups: - name: mixed - action: deny + parameters: + engine: extAuthz + mode: firstMatch rules: - name: shadow-admin dryRun: true match: "request.path.startsWith('/admin')" + validation: + action: deny - name: block-internal match: "request.path.startsWith('/internal')" + validation: + action: deny `, method: "GET", path: "/internal/secret", @@ -358,16 +436,11 @@ groups: } } -// errBody is an io.ReadCloser whose Read always fails, used to simulate a -// request body that cannot be read so we can exercise the read-error path. type errBody struct{} func (errBody) Read([]byte) (int, error) { return 0, io.ErrUnexpectedEOF } func (errBody) Close() error { return nil } -// TestServerReadBodyError verifies the body-read failure path: fail-closed -// (deny) by default, but pass-through (200) under global dry-run. The error -// must be counted and the dry-run header must reflect the mode either way. func TestServerReadBodyError(t *testing.T) { cases := []struct { name string @@ -376,21 +449,34 @@ func TestServerReadBodyError(t *testing.T) { wantDryRun string }{ { - name: "read error fail-closed denies", - policyYAML: `defaults: { action: allow } + name: "read error fail-closed denies", + policyYAML: ` +defaults: + extAuthz: + action: allow groups: - name: g - rules: [{ name: r, match: "true" }] + parameters: + engine: extAuthz + mode: firstMatch + rules: [{ name: r, match: "true", validation: { action: allow } }] `, wantStatus: 403, wantDryRun: "false", }, { - name: "read error under global dryRun passes through", - policyYAML: `defaults: { action: allow, dryRun: true } + name: "read error under global dryRun passes through", + policyYAML: ` +defaults: + extAuthz: + action: allow + dryRun: true groups: - name: g - rules: [{ name: r, match: "true" }] + parameters: + engine: extAuthz + mode: firstMatch + rules: [{ name: r, match: "true", validation: { action: allow } }] `, wantStatus: 200, wantDryRun: "true", @@ -419,3 +505,140 @@ groups: }) } } + +func TestServerBodyOverflow(t *testing.T) { + cases := []struct { + name string + policyYAML string + bodySize int + wantStatus int + wantResult string + wantDryRun string + }{ + { + name: "overflow deny status code 403 and body content", + policyYAML: ` +defaults: + extAuthz: + action: allow + maxBodyBytes: 10 + denyStatus: 403 + denyBody: "Too Large" +groups: + - name: g + parameters: + engine: extAuthz + mode: firstMatch + rules: [{ name: r, match: "true", validation: { action: allow } }] +`, + bodySize: 15, + wantStatus: 403, + wantResult: "deny", + wantDryRun: "false", + }, + { + name: "overflow under global dryRun yields 200", + policyYAML: ` +defaults: + extAuthz: + action: allow + maxBodyBytes: 10 + dryRun: true +groups: + - name: g + parameters: + engine: extAuthz + mode: firstMatch + rules: [{ name: r, match: "true", validation: { action: allow } }] +`, + bodySize: 15, + wantStatus: 200, + wantResult: "deny", + wantDryRun: "true", + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + s := New(loadPolicy(t, tc.policyYAML)) + body := strings.Repeat("A", tc.bodySize) + req := httptest.NewRequest(http.MethodPost, "/anything", strings.NewReader(body)) + rec := httptest.NewRecorder() + + s.handle(rec, req) + + res := rec.Result() + if res.StatusCode != tc.wantStatus { + t.Errorf("status: got %d, want %d", res.StatusCode, tc.wantStatus) + } + if got := res.Header.Get(hdrResult); got != tc.wantResult { + t.Errorf("%s: got %q, want %q", hdrResult, got, tc.wantResult) + } + if got := res.Header.Get(hdrDry); got != tc.wantDryRun { + t.Errorf("%s: got %q, want %q", hdrDry, got, tc.wantDryRun) + } + if got := res.Header.Get(hdrRule); got != "" { + t.Errorf("%s: got %q, want %q", hdrRule, got, "") + } + if got := res.Header.Get(hdrReason); got != "request body too large" { + t.Errorf("%s: got %q, want %q", hdrReason, got, "request body too large") + } + if tc.wantStatus == 403 { + respBody, _ := io.ReadAll(res.Body) + if string(respBody) != "Too Large" { + t.Errorf("expected body %q, got %q", "Too Large", string(respBody)) + } + } + }) + } +} + +// Canary: the HTTP server serves extAuthz correctly even when the policy also +// carries extProc groups. The presence of extProc groups must not break the +// load nor alter the verdict (the HTTP server only runs extAuthz). +func TestServerIgnoresExtProcGroups(t *testing.T) { + cfg := loadPolicy(t, ` +defaults: + extAuthz: + action: deny +groups: + - name: allow-foo + parameters: + engine: extAuthz + mode: firstMatch + rules: + - name: get-foo + match: "request.method == 'GET' && request.path.startsWith('/foo')" + validation: + action: allow + - name: rewrite-redirect + parameters: + engine: extProc + phase: responseHeaders + mode: applyAll + rules: + - name: stamp + match: "true" + mutations: + - op: setHeader + name: x-stamped + value: "'yes'" +`) + s := New(cfg) + ts := httptest.NewServer(http.HandlerFunc(s.handle)) + defer ts.Close() + + res, _ := http.Get(ts.URL + "/foo/bar") + if res.StatusCode != 200 { + t.Fatalf("extAuthz allow must still work with extProc groups present, got %d", res.StatusCode) + } + if res.Header.Get(hdrRule) != "allow-foo/get-foo" { + t.Fatalf("verdict must come from the extAuthz group, got rule %q", res.Header.Get(hdrRule)) + } + + res, _ = http.Get(ts.URL + "/nope") + if res.StatusCode != 403 { + t.Fatalf("non-matching request must fall to default deny, got %d", res.StatusCode) + } +} diff --git a/internal/policy/extproc.go b/internal/policy/extproc.go new file mode 100644 index 0000000..b6afafe --- /dev/null +++ b/internal/policy/extproc.go @@ -0,0 +1,157 @@ +package policy + +import ( + "context" + + "request-validator/internal/celenv" +) + +// ResolvedMutation represents an evaluated extProc traffic modification. +type ResolvedMutation struct { + Op string // setHeader|appendHeader|removeHeader|setBody|setStatus|directResponse + Name string // header ops (vacio en setBody/setStatus) + Value string // setHeader/appendHeader/setBody: valor ya evaluado + Status int // setStatus: codigo ya evaluado + RespStatus int // directResponse: status + RespHeaders map[string]string // directResponse: headers + RespBody string // directResponse: body + Rule string // "group/rule" que origino la mutacion (para logging) + DryRun bool // true si la regla origen tenia dryRun:true +} + +// ProcResult contains the ordered sequence of resolved mutations. +type ProcResult struct { + Mutations []ResolvedMutation // en orden de aplicacion; vacio si nada matcheo +} + +// EvaluateProc processes extProc policies and generates resolved mutations. +// It iterates matching groups and rules based on phase and evaluation mode. +func (c *Config) EvaluateProc(ctx context.Context, phase string, req *Request, resp *Response) ProcResult { + if req == nil { + return ProcResult{} + } + + vars := map[string]any{ + "request": buildRequestVar(req), + "facts": c.snapshot(), + } + if (phase == "responseHeaders" || phase == "responseBody") && resp != nil { + vars["response"] = buildResponseVar(resp) + } + + var resolved []ResolvedMutation + + for gi := range c.Groups { + g := &c.Groups[gi] + if g.Parameters.Engine != "extProc" || g.Parameters.Phase != phase { + continue + } + + if g.matchProg != nil { + ok, err := celenv.Eval(ctx, g.matchProg, vars) + if err != nil || !ok { + continue + } + } + + if g.Parameters.Mode == "firstMatch" { + for ri := range g.Rules { + r := &g.Rules[ri] + ok, err := celenv.Eval(ctx, r.matchProg, vars) + if err != nil { + continue + } + if ok { + resolved = append(resolved, c.resolveMutations(ctx, g, r, vars)...) + break + } + } + } else if g.Parameters.Mode == "applyAll" { + for ri := range g.Rules { + r := &g.Rules[ri] + ok, err := celenv.Eval(ctx, r.matchProg, vars) + if err != nil { + continue + } + if ok { + resolved = append(resolved, c.resolveMutations(ctx, g, r, vars)...) + } + } + } + } + + return ProcResult{ + Mutations: resolved, + } +} + +// resolveMutations evaluates each mutation expression of a matching rule. +// Non-string/non-int or failed evaluations are skipped safely (fail-safe). +func (c *Config) resolveMutations(ctx context.Context, g *Group, r *Rule, vars map[string]any) []ResolvedMutation { + var list []ResolvedMutation + ruleName := g.Name + "/" + r.Name + + for _, m := range r.Mutations { + switch m.Op { + case "directResponse": + hdrs, err := celenv.EvalStringMap(ctx, m.headersProg, vars) + if err != nil { + continue + } + body, err := celenv.EvalString(ctx, m.bodyProg, vars) + if err != nil { + continue + } + list = append(list, ResolvedMutation{ + Op: m.Op, + RespStatus: m.Status, + RespHeaders: hdrs, + RespBody: body, + Rule: ruleName, + DryRun: r.DryRun, + }) + case "setHeader", "appendHeader": + val, err := celenv.EvalString(ctx, m.valueProg, vars) + if err != nil { + continue + } + list = append(list, ResolvedMutation{ + Op: m.Op, + Name: m.Name, + Value: val, + Rule: ruleName, + DryRun: r.DryRun, + }) + case "setBody": + val, err := celenv.EvalString(ctx, m.valueProg, vars) + if err != nil { + continue + } + list = append(list, ResolvedMutation{ + Op: m.Op, + Value: val, + Rule: ruleName, + DryRun: r.DryRun, + }) + case "setStatus": + code, err := celenv.EvalInt(ctx, m.codeProg, vars) + if err != nil { + continue + } + list = append(list, ResolvedMutation{ + Op: m.Op, + Status: int(code), + Rule: ruleName, + DryRun: r.DryRun, + }) + case "removeHeader": + list = append(list, ResolvedMutation{ + Op: m.Op, + Name: m.Name, + Rule: ruleName, + DryRun: r.DryRun, + }) + } + } + return list +} diff --git a/internal/policy/extproc_test.go b/internal/policy/extproc_test.go new file mode 100644 index 0000000..526ac40 --- /dev/null +++ b/internal/policy/extproc_test.go @@ -0,0 +1,600 @@ +package policy + +import ( + "context" + "net/http" + "reflect" + "testing" +) + +// TestBuildResponseVar tests that Response variables map to CEL correctly. +func TestBuildResponseVar(t *testing.T) { + resp := &Response{ + Status: 201, + Headers: http.Header{ + "Content-Type": []string{"application/json"}, + "X-Custom": []string{"foo", "bar"}, + }, + Body: []byte(`{"id": 42, "nested": {"val": "hello"}}`), + } + + got := buildResponseVar(resp) + + if got["status"] != int64(201) { + t.Errorf("expected status 201, got %v", got["status"]) + } + + headers := got["headers"].(map[string][]string) + if !reflect.DeepEqual(headers["x-custom"], []string{"foo", "bar"}) { + t.Errorf("expected headers x-custom, got %v", headers["x-custom"]) + } + + header := got["header"].(map[string]string) + if header["content-type"] != "application/json" { + t.Errorf("expected header content-type application/json, got %q", header["content-type"]) + } + + body := got["body"].(map[string]any) + if body["raw"] != `{"id": 42, "nested": {"val": "hello"}}` { + t.Errorf("expected raw body, got %q", body["raw"]) + } + if body["size"] != int64(38) { + t.Errorf("expected size 38, got %v", body["size"]) + } + if body["contentType"] != "application/json" { + t.Errorf("expected contentType application/json, got %v", body["contentType"]) + } + if body["jsonOk"] != true { + t.Errorf("expected jsonOk true") + } + + jsonMap := body["json"].(map[string]any) + if jsonMap["id"] != float64(42) { + t.Errorf("expected json id 42, got %v", jsonMap["id"]) + } +} + +// TestEvaluateProc_FirstMatch verifies that only the first matching rule applies mutations. +func TestEvaluateProc_FirstMatch(t *testing.T) { + yamlStr := ` +groups: + - name: g1 + parameters: + engine: extProc + mode: firstMatch + phase: requestHeaders + rules: + - name: r1 + match: "request.header['x-id'] == '1'" + mutations: + - op: setHeader + name: x-rule1 + value: "'applied-r1'" + - name: r2 + match: "true" + mutations: + - op: setHeader + name: x-rule2 + value: "'applied-r2'" +` + cfg, err := LoadBytes([]byte(yamlStr)) + if err != nil { + t.Fatalf("failed loading policy: %v", err) + } + + req := &Request{ + Headers: http.Header{"X-Id": []string{"1"}}, + } + + res := cfg.EvaluateProc(context.Background(), "requestHeaders", req, nil) + + if len(res.Mutations) != 1 { + t.Fatalf("expected exactly 1 mutation, got %d: %v", len(res.Mutations), res.Mutations) + } + if res.Mutations[0].Name != "x-rule1" { + t.Errorf("expected mutation x-rule1, got %s", res.Mutations[0].Name) + } +} + +// TestEvaluateProc_FirstMatch_EmptyMutations verifies empty mutations stop the group evaluation. +func TestEvaluateProc_FirstMatch_EmptyMutations(t *testing.T) { + yamlStr := ` +groups: + - name: g1 + parameters: + engine: extProc + mode: firstMatch + phase: requestHeaders + rules: + - name: r1 + match: "request.header['x-id'] == '1'" + mutations: [] + - name: r2 + match: "true" + mutations: + - op: setHeader + name: x-rule2 + value: "'applied-r2'" +` + cfg, err := LoadBytes([]byte(yamlStr)) + if err != nil { + t.Fatalf("failed loading policy: %v", err) + } + + req := &Request{ + Headers: http.Header{"X-Id": []string{"1"}}, + } + + res := cfg.EvaluateProc(context.Background(), "requestHeaders", req, nil) + + if len(res.Mutations) != 0 { + t.Fatalf("expected 0 mutations since firstMatch matched empty mutations, got %v", res.Mutations) + } +} + +// TestEvaluateProc_ApplyAll verifies multiple matching rules apply in order. +func TestEvaluateProc_ApplyAll(t *testing.T) { + yamlStr := ` +groups: + - name: g1 + parameters: + engine: extProc + mode: applyAll + phase: requestHeaders + rules: + - name: r1 + match: "true" + mutations: + - op: setHeader + name: x-header + value: "'value-r1'" + - name: r2 + match: "true" + mutations: + - op: setHeader + name: x-header + value: "'value-r2'" +` + cfg, err := LoadBytes([]byte(yamlStr)) + if err != nil { + t.Fatalf("failed loading policy: %v", err) + } + + req := &Request{} + res := cfg.EvaluateProc(context.Background(), "requestHeaders", req, nil) + + if len(res.Mutations) != 2 { + t.Fatalf("expected 2 mutations, got %d", len(res.Mutations)) + } + + if res.Mutations[0].Name != "x-header" || res.Mutations[0].Value != "value-r1" { + t.Errorf("expected first mutation to be value-r1, got %v", res.Mutations[0]) + } + if res.Mutations[1].Name != "x-header" || res.Mutations[1].Value != "value-r2" { + t.Errorf("expected second mutation to be value-r2 (last write wins), got %v", res.Mutations[1]) + } +} + +// TestEvaluateProc_ResponseHeaders phase with response object properties. +func TestEvaluateProc_ResponseHeaders(t *testing.T) { + yamlStr := ` +groups: + - name: g1 + parameters: + engine: extProc + mode: applyAll + phase: responseHeaders + rules: + - name: r1 + match: "response.status == 200" + mutations: + - op: setHeader + name: x-custom + value: "response.header['x-original'] + '-suffixed'" + - op: setStatus + code: "201" +` + cfg, err := LoadBytes([]byte(yamlStr)) + if err != nil { + t.Fatalf("failed loading policy: %v", err) + } + + req := &Request{} + resp := &Response{ + Status: 200, + Headers: http.Header{"X-Original": []string{"hello"}}, + } + + res := cfg.EvaluateProc(context.Background(), "responseHeaders", req, resp) + + if len(res.Mutations) != 2 { + t.Fatalf("expected 2 mutations, got %d", len(res.Mutations)) + } + + m0 := res.Mutations[0] + if m0.Op != "setHeader" || m0.Name != "x-custom" || m0.Value != "hello-suffixed" { + t.Errorf("unexpected m0: %+v", m0) + } + + m1 := res.Mutations[1] + if m1.Op != "setStatus" || m1.Status != 201 { + t.Errorf("unexpected m1: %+v", m1) + } +} + +// TestEvaluateProc_PhaseFiltering verifies groups outside current phase are skipped. +func TestEvaluateProc_PhaseFiltering(t *testing.T) { + yamlStr := ` +groups: + - name: g_req + parameters: + engine: extProc + mode: firstMatch + phase: requestHeaders + rules: + - name: r_req + match: "true" + mutations: + - op: setHeader + name: x-phase + value: "'request'" + - name: g_resp + parameters: + engine: extProc + mode: firstMatch + phase: responseHeaders + rules: + - name: r_resp + match: "true" + mutations: + - op: setHeader + name: x-phase + value: "'response'" +` + cfg, err := LoadBytes([]byte(yamlStr)) + if err != nil { + t.Fatalf("failed loading policy: %v", err) + } + + req := &Request{} + + res := cfg.EvaluateProc(context.Background(), "requestHeaders", req, nil) + if len(res.Mutations) != 1 || res.Mutations[0].Name != "x-phase" || res.Mutations[0].Value != "request" { + t.Errorf("expected only request group to apply, got: %v", res.Mutations) + } +} + +// TestEvaluateProc_GroupMatchGuard verifies that group is skipped when match is false. +func TestEvaluateProc_GroupMatchGuard(t *testing.T) { + yamlStr := ` +groups: + - name: g1 + parameters: + engine: extProc + mode: applyAll + phase: requestHeaders + match: "request.header['x-enabled'] == 'true'" + rules: + - name: r1 + match: "true" + mutations: + - op: setHeader + name: x-header + value: "'yes'" +` + cfg, err := LoadBytes([]byte(yamlStr)) + if err != nil { + t.Fatalf("failed loading policy: %v", err) + } + + req := &Request{ + Headers: http.Header{"X-Enabled": []string{"false"}}, + } + res := cfg.EvaluateProc(context.Background(), "requestHeaders", req, nil) + + if len(res.Mutations) != 0 { + t.Errorf("expected group to be skipped, but got: %v", res.Mutations) + } +} + +// TestEvaluateProc_DryRun verifies rules carry correct dry-run flag. +func TestEvaluateProc_DryRun(t *testing.T) { + yamlStr := ` +groups: + - name: g1 + parameters: + engine: extProc + mode: applyAll + phase: requestHeaders + rules: + - name: r1 + match: "true" + dryRun: true + mutations: + - op: setHeader + name: x-dry + value: "'dry'" + - name: r2 + match: "true" + dryRun: false + mutations: + - op: setHeader + name: x-wet + value: "'wet'" +` + cfg, err := LoadBytes([]byte(yamlStr)) + if err != nil { + t.Fatalf("failed loading policy: %v", err) + } + + req := &Request{} + res := cfg.EvaluateProc(context.Background(), "requestHeaders", req, nil) + + if len(res.Mutations) != 2 { + t.Fatalf("expected 2 mutations, got %d", len(res.Mutations)) + } + + if !res.Mutations[0].DryRun { + t.Errorf("expected first mutation to have DryRun: true") + } + if res.Mutations[1].DryRun { + t.Errorf("expected second mutation to have DryRun: false") + } +} + +// TestEvaluateProc_FailSafe verifies mutation is omitted when dynamic CEL eval fails. +func TestEvaluateProc_FailSafe(t *testing.T) { + yamlStr := ` +groups: + - name: g1 + parameters: + engine: extProc + mode: applyAll + phase: requestHeaders + rules: + - name: r1 + match: "true" + mutations: + - op: setHeader + name: x-first + value: "'first'" + - op: setHeader + name: x-failing + value: "request.headers['non-existent'][10]" + - op: setHeader + name: x-last + value: "'last'" +` + cfg, err := LoadBytes([]byte(yamlStr)) + if err != nil { + t.Fatalf("failed loading policy: %v", err) + } + + req := &Request{} + res := cfg.EvaluateProc(context.Background(), "requestHeaders", req, nil) + + if len(res.Mutations) != 2 { + t.Fatalf("expected exactly 2 mutations after omitting the failed one, got %d: %v", len(res.Mutations), res.Mutations) + } + + if res.Mutations[0].Name != "x-first" || res.Mutations[1].Name != "x-last" { + t.Errorf("unexpected mutations sequence: %v", res.Mutations) + } +} + +func TestDirectResponse_Success(t *testing.T) { + yamlStr := ` +groups: + - name: g1 + parameters: + engine: extProc + mode: firstMatch + phase: responseHeaders + rules: + - name: r1 + match: "response.status == 302" + mutations: + - op: directResponse + status: 200 + headers: '{"content-type": "text/html", "x-orig-location": response.header["location"]}' + body: '"hi redirect to " + response.header["location"]' +` + cfg, err := LoadBytes([]byte(yamlStr)) + if err != nil { + t.Fatalf("failed loading policy: %v", err) + } + + req := &Request{} + resp := &Response{ + Status: 302, + Headers: http.Header{"Location": []string{"https://example.com"}}, + } + + res := cfg.EvaluateProc(context.Background(), "responseHeaders", req, resp) + + if len(res.Mutations) != 1 { + t.Fatalf("expected exactly 1 mutation, got %d", len(res.Mutations)) + } + + m := res.Mutations[0] + if m.Op != "directResponse" { + t.Errorf("expected op directResponse, got %q", m.Op) + } + if m.RespStatus != 200 { + t.Errorf("expected RespStatus 200, got %d", m.RespStatus) + } + expectedHeaders := map[string]string{ + "content-type": "text/html", + "x-orig-location": "https://example.com", + } + if !reflect.DeepEqual(m.RespHeaders, expectedHeaders) { + t.Errorf("expected RespHeaders %v, got %v", expectedHeaders, m.RespHeaders) + } + if m.RespBody != "hi redirect to https://example.com" { + t.Errorf("expected RespBody, got %q", m.RespBody) + } +} + +func TestDirectResponse_HeadersFromResponse(t *testing.T) { + yamlStr := ` +groups: + - name: g1 + parameters: + engine: extProc + mode: firstMatch + phase: responseHeaders + rules: + - name: r1 + match: "true" + mutations: + - op: directResponse + status: 200 + headers: 'response.header' + body: '"body"' +` + cfg, err := LoadBytes([]byte(yamlStr)) + if err != nil { + t.Fatalf("failed loading policy: %v", err) + } + + req := &Request{} + resp := &Response{ + Status: 200, + Headers: http.Header{"X-Test-Header": []string{"some-value"}}, + } + + res := cfg.EvaluateProc(context.Background(), "responseHeaders", req, resp) + + if len(res.Mutations) != 1 { + t.Fatalf("expected exactly 1 mutation, got %d", len(res.Mutations)) + } + + m := res.Mutations[0] + if m.RespHeaders["x-test-header"] != "some-value" { + t.Errorf("expected headers to carry x-test-header: some-value, got %v", m.RespHeaders) + } +} + +func TestDirectResponse_FailSafe_Canary(t *testing.T) { + yamlStr := ` +groups: + - name: g1 + parameters: + engine: extProc + mode: firstMatch + phase: responseHeaders + rules: + - name: r1 + match: "true" + mutations: + - op: directResponse + status: 200 + headers: 'response.headers' + body: '"body"' +` + cfg, err := LoadBytes([]byte(yamlStr)) + if err != nil { + t.Fatalf("failed loading policy: %v", err) + } + + req := &Request{} + resp := &Response{ + Status: 200, + Headers: http.Header{"X-Test-Header": []string{"some-value"}}, + } + + res := cfg.EvaluateProc(context.Background(), "responseHeaders", req, resp) + + if len(res.Mutations) != 0 { + t.Fatalf("expected directResponse to be omitted due to type mismatch in headers, but got: %v", res.Mutations) + } +} + +// Canary: in a policy mixing both engines, each engine must only see its own +// groups. extAuthz Evaluate must ignore extProc groups (and never treat them +// as deciding), and EvaluateProc must ignore extAuthz groups. A regression +// that lets one engine pick up the other's groups breaks this. +func TestEngineIsolation_MixedPolicy(t *testing.T) { + yamlStr := ` +defaults: + extAuthz: + action: deny +groups: + - name: authz-allow-internal + parameters: + engine: extAuthz + mode: firstMatch + rules: + - name: allow-it + match: "request.path == '/ok'" + validation: + action: allow + - name: proc-stamp-response + parameters: + engine: extProc + phase: responseHeaders + mode: applyAll + rules: + - name: stamp + match: "true" + mutations: + - op: setHeader + name: x-stamped + value: "'yes'" +` + cfg, err := LoadBytes([]byte(yamlStr)) + if err != nil { + t.Fatalf("load mixed policy: %v", err) + } + + // extAuthz side: the extProc group must not interfere with the verdict. + allow := cfg.Evaluate(context.Background(), &Request{Path: "/ok"}) + if !allow.Allowed || allow.Rule != "authz-allow-internal/allow-it" { + t.Fatalf("extAuthz should allow via its own group only, got %+v", allow) + } + deny := cfg.Evaluate(context.Background(), &Request{Path: "/other"}) + if deny.Allowed || deny.Rule != "" { + t.Fatalf("extAuthz should fall to defaults, never decide via an extProc group, got %+v", deny) + } + + // extProc side: only the extProc group runs; the extAuthz group is invisible. + res := cfg.EvaluateProc(context.Background(), "responseHeaders", + &Request{Path: "/ok"}, &Response{Status: 200, Headers: http.Header{}}) + if len(res.Mutations) != 1 || res.Mutations[0].Name != "x-stamped" { + t.Fatalf("extProc should run only its own group, got %+v", res.Mutations) + } +} + +// Canary: EvaluateProc must only run groups bound to the requested phase. A +// responseHeaders group must not fire when Envoy is at the requestHeaders phase. +func TestEvaluateProc_OnlyRunsRequestedPhase(t *testing.T) { + yamlStr := ` +groups: + - name: on-response + parameters: + engine: extProc + phase: responseHeaders + mode: applyAll + rules: + - name: stamp + match: "true" + mutations: + - op: setHeader + name: x-resp + value: "'1'" +` + cfg, err := LoadBytes([]byte(yamlStr)) + if err != nil { + t.Fatalf("load: %v", err) + } + + atRequest := cfg.EvaluateProc(context.Background(), "requestHeaders", &Request{}, nil) + if len(atRequest.Mutations) != 0 { + t.Fatalf("responseHeaders group must not fire at requestHeaders phase, got %+v", atRequest.Mutations) + } + atResponse := cfg.EvaluateProc(context.Background(), "responseHeaders", + &Request{}, &Response{Status: 200, Headers: http.Header{}}) + if len(atResponse.Mutations) != 1 { + t.Fatalf("responseHeaders group must fire at its own phase, got %+v", atResponse.Mutations) + } +} diff --git a/internal/policy/policy.go b/internal/policy/policy.go index 2c7d371..52a69ec 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -1,17 +1,3 @@ -// Package policy contains the configuration model, parser and evaluator that -// drive the request-validator authorisation engine. -// -// The policy is a list of named groups. Each rule has one CEL expression -// (`match`) that decides whether the rule applies; if it does, the rule's -// effective `action` (allow|deny) becomes the verdict. The group's `mode` -// composes the rules: -// -// - `firstMatch` (default): the first rule whose `match` is true wins. -// - `all`: every rule must hold; the first failure -// produces a deny. -// -// A rule inherits its `action` from the group when it does not declare one, -// which lets a "deny-by-default" group be expressed concisely. package policy import ( @@ -33,149 +19,104 @@ import ( // Config is the top-level configuration loaded from YAML. type Config struct { - Defaults Defaults `yaml:"defaults"` - Logging Logging `yaml:"logging"` - Facts []facts.Spec `yaml:"facts"` - Groups []Group `yaml:"groups"` - - // compiled state (set during Load) + Defaults Defaults `yaml:"defaults"` + Logging Logging `yaml:"logging"` + Facts []facts.Spec `yaml:"facts"` + Groups []Group `yaml:"groups"` env *celenv.Env registry *facts.Registry } -// Defaults are server-wide knobs. +// Defaults are server-wide knobs split per-engine. type Defaults struct { - // Action returned when no group decides. "allow" or "deny" (default deny). - Action string `yaml:"action"` - - // DenyStatus is the HTTP status code returned on deny. Default 403. - DenyStatus int `yaml:"denyStatus"` - - // DenyBody is the body returned on deny. Default "Forbidden". - DenyBody string `yaml:"denyBody"` + ExtAuthz ExtAuthzDefaults `yaml:"extAuthz"` + ExtProc ExtProcDefaults `yaml:"extProc"` + DryRun bool `yaml:"dryRun"` +} - // MaxBodyBytes is the maximum number of bytes buffered from the request - // body. Accepts unit suffixes ("1MiB", "512KiB", "1MB"). Default 1 MiB. +// ExtAuthzDefaults configures the HTTP ext-authz engine defaults. +type ExtAuthzDefaults struct { + Action string `yaml:"action"` + DenyStatus int `yaml:"denyStatus"` + DenyBody string `yaml:"denyBody"` + AllowOnError bool `yaml:"allowOnError"` MaxBodyBytes BytesSize `yaml:"maxBodyBytes"` +} - // AllowOnError: if true, evaluation errors produce allow; else deny. - AllowOnError bool `yaml:"allowOnError"` - - // DryRun is the global shadow switch: the service evaluates every request - // normally but never enforces a deny. All requests pass through to Envoy - // with HTTP 200, while the access log and metrics record the real verdict - // so an operator can observe what would be denied before enabling - // enforcement. Default false. - DryRun bool `yaml:"dryRun"` +// ExtProcDefaults configures the gRPC ext_proc engine defaults. +type ExtProcDefaults struct { + MaxBodyBytes BytesSize `yaml:"maxBodyBytes"` + OnBodyOverflow string `yaml:"onBodyOverflow"` } // Logging configures the access-log enrichment applied to every request. -// -// The whole struct is optional; defaults are biased towards "log everything -// safely" - every request produces a line, body and short header values -// are kept, and a small allow-list of obvious secrets is redacted or -// excluded. type Logging struct { - // Level is the global threshold (debug|info|warn|error). Default info. - // Overridden by the --log-level flag if it is non-empty. - Level string `yaml:"level"` - - // Format selects the slog handler: "json" (default) or "console". - // Overridden by the --log-format flag if it is non-empty. - Format string `yaml:"format"` - - // LogBody, when true, includes the request body (truncated to - // defaults.maxBodyBytes already) as a string under request.body in the - // log record. Default false because DCR payloads carry client_secrets. - LogBody bool `yaml:"logBody"` - - // ExcludeHeaders lists header names that must never appear in the log. - // Case-insensitive. Default: ["cookie", "set-cookie"]. - ExcludeHeaders []string `yaml:"excludeHeaders"` - - // RedactHeaders lists header names whose values must be masked. Each - // value is shown as a short prefix followed by '*' characters; values - // shorter than RedactReveal are fully masked. - // Case-insensitive. Default: - // ["authorization","proxy-authorization","x-api-key","x-auth-token"]. - RedactHeaders []string `yaml:"redactHeaders"` - - // RedactReveal is the number of leading characters to keep when - // redacting. Default 6. Set to 0 to always fully mask. - RedactReveal int `yaml:"redactReveal"` - - // RedactQueryParams behaves like RedactHeaders for URL query parameters. - // Default: ["access_token","id_token","code"]. + Level string `yaml:"level"` + Format string `yaml:"format"` + LogBody bool `yaml:"logBody"` + ExcludeHeaders []string `yaml:"excludeHeaders"` + RedactHeaders []string `yaml:"redactHeaders"` + RedactReveal int `yaml:"redactReveal"` RedactQueryParams []string `yaml:"redactQueryParams"` } -// Group is a named collection of rules with a common scope and a mode. +// Group is a named collection of rules bound to an engine with parameters. type Group struct { - Name string `yaml:"name"` - Description string `yaml:"description"` - - // Mode controls how the group composes its rules: - // "firstMatch" (default): the first rule whose `match` is true decides. - // "all": every rule must match; one failure denies. - Mode string `yaml:"mode"` - - // Action is the verdict declared at the group level. Each rule inherits - // it unless overridden. Allowed values: "allow" | "deny". Default "allow". - Action string `yaml:"action"` - - // Match is an optional CEL boolean expression. If present and false the - // whole group is skipped silently. - Match string `yaml:"match"` - - // Rules in declaration order. - Rules []Rule `yaml:"rules"` + Name string `yaml:"name"` + Description string `yaml:"description"` + Parameters Parameters `yaml:"parameters"` + Match string `yaml:"match"` + Rules []Rule `yaml:"rules"` + matchProg cel.Program +} - // compiled state - matchProg cel.Program +// Parameters defines engine-specific wiring and modes. +type Parameters struct { + Engine string `yaml:"engine"` + Mode string `yaml:"mode"` + Phase string `yaml:"phase"` } -// Rule is one entry in `rules:`. +// Rule is one conditional entry in a group. type Rule struct { - Name string `yaml:"name"` - Description string `yaml:"description"` + Name string `yaml:"name"` + Description string `yaml:"description"` + Match string `yaml:"match"` + Validation *Validation `yaml:"validation"` + Mutations []Mutation `yaml:"mutations"` + DryRun bool `yaml:"dryRun"` + matchProg cel.Program +} - // Action overrides the group's action for this specific rule. - // Allowed values: "allow" | "deny" | "" (inherit from group). +// Validation defines the verdict action on a successful rule match. +type Validation struct { Action string `yaml:"action"` +} - // Match is the CEL boolean expression that decides whether the rule - // applies. If omitted, the default is `true` (rule always applies, useful - // as a fallback inside a firstMatch group). - Match string `yaml:"match"` - - // Fallthrough controls what happens when `match` is false in a - // firstMatch group: - // "next" (default in firstMatch): try the next rule. - // "allow" or "deny": emit that verdict immediately. - // Ignored in `all` mode (a failure there is a group-level verdict). - Fallthrough string `yaml:"fallthrough"` - - // DryRun: the rule still logs and emits metrics, but the verdict is - // suppressed (request is allowed). Useful for shadow-testing future - // tightenings. - DryRun bool `yaml:"dryRun"` - - // resolved/compiled state - effectiveAction string - matchProg cel.Program +// Mutation defines header, body or status transformations for ext_proc. +type Mutation struct { + Op string `yaml:"op"` + Name string `yaml:"name"` + Value string `yaml:"value"` + Code string `yaml:"code"` + Status int `yaml:"status"` // directResponse: status literal + Headers string `yaml:"headers"` // directResponse: CEL expression -> map + Body string `yaml:"body"` // directResponse: CEL expression -> string + valueProg cel.Program + codeProg cel.Program + headersProg cel.Program + bodyProg cel.Program } -// Decision is the evaluator output. +// Decision is the HTTP ext-authz evaluator output. type Decision struct { Allowed bool - Rule string // "" if nothing matched, "group/rule" otherwise + Rule string Reason string DryRun bool } -// Request is the normalised view of the incoming HTTP request that the -// httpserver layer hands over to the evaluator. The evaluator turns it into -// a CEL activation map shaped like the README documents. +// Request is the normalised view of the incoming HTTP request. type Request struct { Method string Scheme string @@ -187,6 +128,13 @@ type Request struct { Body []byte } +// Response is the normalised view of the upstream HTTP response. +type Response struct { + Status int + Headers http.Header + Body []byte +} + // LoadFile reads, parses, validates and compiles a policy file. func LoadFile(path string) (*Config, error) { raw, err := os.ReadFile(path) @@ -197,7 +145,7 @@ func LoadFile(path string) (*Config, error) { return LoadBytes(raw) } -// LoadBytes is LoadFile from already-read bytes (useful in tests). +// LoadBytes is LoadFile from already-read bytes. func LoadBytes(b []byte) (*Config, error) { c := &Config{} if err := yamlv3.Unmarshal(b, c); err != nil { @@ -223,9 +171,7 @@ func LoadBytes(b []byte) (*Config, error) { return c, nil } -// Start activates the facts registry: file sources are read, URL fetchers -// perform their first fetch and start refreshing in the background. It is -// safe to call multiple times only after Stop(). +// Start activates the facts registry. func (c *Config) Start(ctx context.Context) error { if c.registry == nil { return nil @@ -233,7 +179,7 @@ func (c *Config) Start(ctx context.Context) error { return c.registry.Start(ctx) } -// Stop cancels the facts registry goroutines and waits for them. +// Stop cancels the facts registry background tasks. func (c *Config) Stop() { if c.registry == nil { return @@ -241,18 +187,25 @@ func (c *Config) Stop() { c.registry.Stop() } +// applyDefaults populates missing config fields with default values. func applyDefaults(c *Config) { - if c.Defaults.Action == "" { - c.Defaults.Action = "deny" + if c.Defaults.ExtAuthz.Action == "" { + c.Defaults.ExtAuthz.Action = "deny" } - if c.Defaults.DenyStatus == 0 { - c.Defaults.DenyStatus = 403 + if c.Defaults.ExtAuthz.DenyStatus == 0 { + c.Defaults.ExtAuthz.DenyStatus = 403 } - if c.Defaults.DenyBody == "" { - c.Defaults.DenyBody = "Forbidden" + if c.Defaults.ExtAuthz.DenyBody == "" { + c.Defaults.ExtAuthz.DenyBody = "Forbidden" } - if c.Defaults.MaxBodyBytes == 0 { - c.Defaults.MaxBodyBytes = 1 << 20 + if c.Defaults.ExtAuthz.MaxBodyBytes == 0 { + c.Defaults.ExtAuthz.MaxBodyBytes = 1 << 20 + } + if c.Defaults.ExtProc.MaxBodyBytes == 0 { + c.Defaults.ExtProc.MaxBodyBytes = 1 << 20 + } + if c.Defaults.ExtProc.OnBodyOverflow == "" { + c.Defaults.ExtProc.OnBodyOverflow = "fail" } if c.Logging.Level == "" { c.Logging.Level = "info" @@ -277,108 +230,267 @@ func applyDefaults(c *Config) { if c.Logging.RedactQueryParams == nil { c.Logging.RedactQueryParams = []string{"access_token", "id_token", "code"} } - for gi := range c.Groups { - g := &c.Groups[gi] - if g.Mode == "" { - g.Mode = "firstMatch" - } - if g.Action == "" { - g.Action = "allow" - } - for ri := range g.Rules { - r := &g.Rules[ri] - r.effectiveAction = r.Action - if r.effectiveAction == "" { - r.effectiveAction = g.Action - } - if r.Fallthrough == "" { - // In firstMatch groups the natural default is "next" so the - // next rule can try. In `all` groups fallthrough is ignored. - r.Fallthrough = "next" - } - } - } } +// validate ensures structural correctness of groups and mutations. func (c *Config) validate() error { - if c.Defaults.Action != "allow" && c.Defaults.Action != "deny" { - return fmt.Errorf("defaults.action must be allow|deny") + if c.Defaults.ExtAuthz.Action != "allow" && c.Defaults.ExtAuthz.Action != "deny" { + return fmt.Errorf("defaults.extAuthz.action must be allow|deny, got %q", c.Defaults.ExtAuthz.Action) + } + if c.Defaults.ExtProc.OnBodyOverflow != "skip" && c.Defaults.ExtProc.OnBodyOverflow != "fail" { + return fmt.Errorf("defaults.extProc.onBodyOverflow must be skip|fail, got %q", c.Defaults.ExtProc.OnBodyOverflow) } - seen := map[string]bool{} + + if len(c.Groups) == 0 { + return fmt.Errorf("policy must contain at least one group") + } + + seenGroups := make(map[string]bool) + for gi, g := range c.Groups { if g.Name == "" { - return fmt.Errorf("groups[%d]: missing name", gi) + return fmt.Errorf("group at index %d is missing 'name'", gi) } - if seen[g.Name] { + if seenGroups[g.Name] { return fmt.Errorf("duplicate group name %q", g.Name) } - seen[g.Name] = true - if g.Mode != "firstMatch" && g.Mode != "all" { - return fmt.Errorf("group %q: mode must be firstMatch|all", g.Name) + seenGroups[g.Name] = true + + if g.Parameters.Engine != "extAuthz" && g.Parameters.Engine != "extProc" { + return fmt.Errorf("group %q: parameters.engine must be extAuthz|extProc, got %q", g.Name, g.Parameters.Engine) } - if g.Action != "allow" && g.Action != "deny" { - return fmt.Errorf("group %q: action must be allow|deny", g.Name) + + if g.Parameters.Engine == "extAuthz" { + if g.Parameters.Mode != "firstMatch" && g.Parameters.Mode != "matchAll" { + return fmt.Errorf("group %q (extAuthz): parameters.mode must be firstMatch|matchAll, got %q", g.Name, g.Parameters.Mode) + } + } else { + if g.Parameters.Mode != "firstMatch" && g.Parameters.Mode != "applyAll" { + return fmt.Errorf("group %q (extProc): parameters.mode must be firstMatch|applyAll, got %q", g.Name, g.Parameters.Mode) + } + } + + if g.Parameters.Engine == "extProc" { + phase := g.Parameters.Phase + if phase != "requestHeaders" && phase != "requestBody" && phase != "responseHeaders" && phase != "responseBody" { + return fmt.Errorf("group %q (extProc): parameters.phase must be requestHeaders|requestBody|responseHeaders|responseBody, got %q", g.Name, phase) + } + } else { + if g.Parameters.Phase != "" { + return fmt.Errorf("group %q (extAuthz): parameters.phase is forbidden, got %q", g.Name, g.Parameters.Phase) + } } + if len(g.Rules) == 0 { return fmt.Errorf("group %q: must contain at least one rule", g.Name) } + + seenRules := make(map[string]bool) for ri, r := range g.Rules { if r.Name == "" { - return fmt.Errorf("group %q rules[%d]: missing name", g.Name, ri) + return fmt.Errorf("group %q rule at index %d is missing 'name'", g.Name, ri) } - if seen[g.Name+"/"+r.Name] { - return fmt.Errorf("duplicate rule name %q in group %q", r.Name, g.Name) + if seenRules[r.Name] { + return fmt.Errorf("group %q: duplicate rule name %q", g.Name, r.Name) } - seen[g.Name+"/"+r.Name] = true - ea := r.effectiveAction - if ea != "allow" && ea != "deny" { - return fmt.Errorf("rule %q/%q: action must be allow|deny (got %q)", g.Name, r.Name, ea) + seenRules[r.Name] = true + + if strings.TrimSpace(r.Match) == "" { + return fmt.Errorf("rule %q/%q: match expression is mandatory and cannot be empty", g.Name, r.Name) } - if r.Fallthrough != "next" && r.Fallthrough != "allow" && r.Fallthrough != "deny" { - return fmt.Errorf("rule %q/%q: fallthrough must be next|allow|deny", g.Name, r.Name) + + if g.Parameters.Engine == "extAuthz" { + if r.Validation == nil { + return fmt.Errorf("rule %q/%q (extAuthz): validation block is mandatory", g.Name, r.Name) + } + if len(r.Mutations) > 0 { + return fmt.Errorf("rule %q/%q (extAuthz): mutations are forbidden", g.Name, r.Name) + } + action := r.Validation.Action + if action != "allow" && action != "deny" { + return fmt.Errorf("rule %q/%q (extAuthz): validation.action must be allow|deny, got %q", g.Name, r.Name, action) + } + if g.Parameters.Mode == "matchAll" && action != "allow" { + return fmt.Errorf("rule %q/%q (extAuthz under matchAll): validation.action must be allow, got %q", g.Name, r.Name, action) + } + + } else { + if r.Validation != nil { + return fmt.Errorf("rule %q/%q (extProc): validation block is forbidden", g.Name, r.Name) + } + hasDirect := false + for _, m := range r.Mutations { + if m.Op == "directResponse" { + hasDirect = true + } + } + if hasDirect && len(r.Mutations) > 1 { + return fmt.Errorf("rule %q/%q: directResponse must be the only mutation in the rule", g.Name, r.Name) + } + for mi, m := range r.Mutations { + switch m.Op { + case "directResponse": + if m.Status <= 0 || m.Status > 599 { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): status is required, must be a valid HTTP code (got %d)", g.Name, r.Name, mi, m.Op, m.Status) + } + if strings.TrimSpace(m.Headers) == "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): headers is required and cannot be empty", g.Name, r.Name, mi, m.Op) + } + if strings.TrimSpace(m.Body) == "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): body is required and cannot be empty", g.Name, r.Name, mi, m.Op) + } + if m.Name != "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): name must be empty", g.Name, r.Name, mi, m.Op) + } + if m.Value != "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): value must be empty", g.Name, r.Name, mi, m.Op) + } + if m.Code != "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): code must be empty", g.Name, r.Name, mi, m.Op) + } + phase := g.Parameters.Phase + if phase != "responseHeaders" && phase != "responseBody" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): directResponse is only legal in responseHeaders|responseBody phases, got %q", g.Name, r.Name, mi, m.Op, phase) + } + case "setHeader", "appendHeader": + if strings.TrimSpace(m.Name) == "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): name is required and cannot be empty", g.Name, r.Name, mi, m.Op) + } + if strings.TrimSpace(m.Value) == "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): value is required and cannot be empty", g.Name, r.Name, mi, m.Op) + } + if m.Code != "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): code must be empty", g.Name, r.Name, mi, m.Op) + } + case "removeHeader": + if strings.TrimSpace(m.Name) == "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): name is required and cannot be empty", g.Name, r.Name, mi, m.Op) + } + if m.Value != "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): value must be empty", g.Name, r.Name, mi, m.Op) + } + if m.Code != "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): code must be empty", g.Name, r.Name, mi, m.Op) + } + case "setBody": + if strings.TrimSpace(m.Value) == "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): value is required and cannot be empty", g.Name, r.Name, mi, m.Op) + } + if m.Name != "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): name must be empty", g.Name, r.Name, mi, m.Op) + } + if m.Code != "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): code must be empty", g.Name, r.Name, mi, m.Op) + } + phase := g.Parameters.Phase + if phase != "requestBody" && phase != "responseBody" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): setBody is only legal in requestBody|responseBody phases, got %q", g.Name, r.Name, mi, m.Op, phase) + } + case "setStatus": + if strings.TrimSpace(m.Code) == "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): code is required and cannot be empty", g.Name, r.Name, mi, m.Op) + } + if m.Name != "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): name must be empty", g.Name, r.Name, mi, m.Op) + } + if m.Value != "" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): value must be empty", g.Name, r.Name, mi, m.Op) + } + phase := g.Parameters.Phase + if phase != "responseHeaders" && phase != "responseBody" { + return fmt.Errorf("rule %q/%q mutation[%d] (%s): setStatus is only legal in responseHeaders|responseBody phases, got %q", g.Name, r.Name, mi, m.Op, phase) + } + default: + return fmt.Errorf("rule %q/%q mutation[%d]: unknown op %q", g.Name, r.Name, mi, m.Op) + } + } } } } return nil } +// compile compiles all CEL expressions inside groups with proper scopes. func (c *Config) compile() error { for gi := range c.Groups { g := &c.Groups[gi] + + var scope celenv.Scope + if g.Parameters.Engine == "extAuthz" { + scope = celenv.ScopeRequest + } else { + switch g.Parameters.Phase { + case "requestHeaders", "requestBody": + scope = celenv.ScopeRequest + case "responseHeaders", "responseBody": + scope = celenv.ScopeResponse + } + } + if strings.TrimSpace(g.Match) != "" { - p, err := c.env.Compile(g.Match) + p, err := c.env.Compile(g.Match, scope) if err != nil { return fmt.Errorf("group %q match: %w", g.Name, err) } g.matchProg = p } + for ri := range g.Rules { r := &g.Rules[ri] - src := strings.TrimSpace(r.Match) - if src == "" { - src = "true" - } - p, err := c.env.Compile(src) + p, err := c.env.Compile(r.Match, scope) if err != nil { return fmt.Errorf("rule %q/%q match: %w", g.Name, r.Name, err) } r.matchProg = p + + for mi := range r.Mutations { + m := &r.Mutations[mi] + switch m.Op { + case "directResponse": + hp, err := c.env.CompileStringMap(m.Headers, scope) + if err != nil { + return fmt.Errorf("rule %q/%q mutation %d headers (%s): %w", g.Name, r.Name, mi, m.Op, err) + } + m.headersProg = hp + bp, err := c.env.CompileString(m.Body, scope) + if err != nil { + return fmt.Errorf("rule %q/%q mutation %d body (%s): %w", g.Name, r.Name, mi, m.Op, err) + } + m.bodyProg = bp + case "setHeader", "appendHeader", "setBody": + vp, err := c.env.CompileString(m.Value, scope) + if err != nil { + return fmt.Errorf("rule %q/%q mutation %d value (%s): %w", g.Name, r.Name, mi, m.Op, err) + } + m.valueProg = vp + case "setStatus": + cp, err := c.env.CompileInt(m.Code, scope) + if err != nil { + return fmt.Errorf("rule %q/%q mutation %d code (%s): %w", g.Name, r.Name, mi, m.Op, err) + } + m.codeProg = cp + } + } } } return nil } -// Evaluate runs the policy against a request. +// Evaluate runs the ext-authz policy against a request. func (c *Config) Evaluate(ctx context.Context, r *Request) Decision { requestVar := buildRequestVar(r) factsVar := c.snapshot() for gi := range c.Groups { - if d, decided := c.evalGroup(ctx, &c.Groups[gi], requestVar, factsVar); decided { + g := &c.Groups[gi] + if g.Parameters.Engine != "extAuthz" { + continue + } + if d, decided := c.evalGroup(ctx, g, requestVar, factsVar); decided { return d } } return Decision{ - Allowed: c.Defaults.Action == "allow", + Allowed: c.Defaults.ExtAuthz.Action == "allow", Rule: "", Reason: "no group matched", } @@ -392,9 +504,13 @@ func (c *Config) snapshot() map[string]any { return c.registry.Snapshot() } +// evalGroup runs match and rules inside a single ext-authz group. func (c *Config) evalGroup(ctx context.Context, g *Group, requestVar, factsVar map[string]any) (Decision, bool) { if g.matchProg != nil { - ok, err := celenv.Eval(ctx, g.matchProg, requestVar, factsVar) + ok, err := celenv.Eval(ctx, g.matchProg, map[string]any{ + "request": requestVar, + "facts": factsVar, + }) if err != nil { return c.errorDecision(g.Name, "group match error: "+err.Error(), false), true } @@ -403,75 +519,65 @@ func (c *Config) evalGroup(ctx context.Context, g *Group, requestVar, factsVar m } } - switch g.Mode { - case "all": + switch g.Parameters.Mode { + case "matchAll": for ri := range g.Rules { r := &g.Rules[ri] - ok, err := celenv.Eval(ctx, r.matchProg, requestVar, factsVar) + ok, err := celenv.Eval(ctx, r.matchProg, map[string]any{ + "request": requestVar, + "facts": factsVar, + }) if err != nil { return c.errorDecision(g.Name+"/"+r.Name, "match error: "+err.Error(), r.DryRun), true } - // In `all` mode, every rule must produce its declared verdict; - // if it doesn't match, the group denies (or allows when the - // rule's action is deny - interpreting "all of these denies must - // hold" naturally). - ruleAllow := r.effectiveAction == "allow" - if ok != ruleAllow { - // Group fails. Emit the opposite of the group action. - groupAllow := g.Action == "allow" + if !ok { return Decision{ - Allowed: !groupAllow, + Allowed: false, Rule: g.Name + "/" + r.Name, - Reason: fmt.Sprintf("group mode=all: rule %q failed", r.Name), + Reason: fmt.Sprintf("matchAll: rule %s failed", r.Name), DryRun: r.DryRun, }, true } } - // All rules held. return Decision{ - Allowed: g.Action == "allow", + Allowed: true, Rule: g.Name, - Reason: "group mode=all: every rule held", + Reason: "matchAll: all rules held", + DryRun: false, }, true - default: // firstMatch + default: for ri := range g.Rules { r := &g.Rules[ri] - ok, err := celenv.Eval(ctx, r.matchProg, requestVar, factsVar) + ok, err := celenv.Eval(ctx, r.matchProg, map[string]any{ + "request": requestVar, + "facts": factsVar, + }) if err != nil { return c.errorDecision(g.Name+"/"+r.Name, "match error: "+err.Error(), r.DryRun), true } if ok { return Decision{ - Allowed: r.effectiveAction == "allow", + Allowed: r.Validation.Action == "allow", Rule: g.Name + "/" + r.Name, Reason: "matched", DryRun: r.DryRun, }, true } - // match=false; apply fallthrough. - switch r.Fallthrough { - case "next": - continue - case "allow": - return Decision{Allowed: true, Rule: g.Name + "/" + r.Name, Reason: "no match -> fallthrough allow", DryRun: r.DryRun}, true - case "deny": - return Decision{Allowed: false, Rule: g.Name + "/" + r.Name, Reason: "no match -> fallthrough deny", DryRun: r.DryRun}, true - } } return Decision{}, false } } +// errorDecision converts a CEL runtime evaluation error into a Decision. func (c *Config) errorDecision(rule, reason string, dry bool) Decision { - if c.Defaults.AllowOnError { + if c.Defaults.ExtAuthz.AllowOnError { return Decision{Allowed: true, Rule: rule, Reason: reason + " (allowOnError)", DryRun: dry} } return Decision{Allowed: false, Rule: rule, Reason: reason, DryRun: dry} } -// buildRequestVar projects a Request into the dict-shaped value that policies -// see as the `request` CEL variable. +// buildRequestVar projects a Request into the dict-shaped value for CEL request variable. func buildRequestVar(r *Request) map[string]any { headersAll := map[string][]string{} headersFirst := map[string]string{} @@ -530,6 +636,50 @@ func buildRequestVar(r *Request) map[string]any { } } +// buildResponseVar projects a Response into the dict-shaped value for CEL response variable. +func buildResponseVar(resp *Response) map[string]any { + if resp == nil { + return map[string]any{} + } + headersAll := map[string][]string{} + headersFirst := map[string]string{} + for k, vs := range resp.Headers { + lk := strings.ToLower(k) + headersAll[lk] = vs + if len(vs) > 0 { + headersFirst[lk] = vs[0] + } + } + + body := map[string]any{ + "raw": string(resp.Body), + "size": int64(len(resp.Body)), + "contentType": headersFirst["content-type"], + } + if jv, ok := tryJSON(resp.Body); ok { + body["json"] = jv + body["jsonOk"] = true + } else { + body["json"] = map[string]any{} + body["jsonOk"] = false + } + if yv, ok := tryYAML(resp.Body); ok { + body["yaml"] = yv + body["yamlOk"] = true + } else { + body["yaml"] = map[string]any{} + body["yamlOk"] = false + } + + return map[string]any{ + "status": int64(resp.Status), + "headers": headersAll, + "header": headersFirst, + "body": body, + } +} + +// tryJSON tries to decode json, normalising decoded nested structures. func tryJSON(b []byte) (any, bool) { if len(b) == 0 { return nil, false @@ -541,6 +691,7 @@ func tryJSON(b []byte) (any, bool) { return normaliseDecoded(v), true } +// tryYAML tries to decode yaml, normalising decoded nested structures. func tryYAML(b []byte) (any, bool) { if len(b) == 0 { return nil, false @@ -549,14 +700,10 @@ func tryYAML(b []byte) (any, bool) { if err := yamlv3.Unmarshal(b, &v); err != nil { return nil, false } - // yaml.v3 may decode maps as map[string]any already, but sometimes as - // map[any]any. Normalise so CEL sees consistent types. return normaliseDecoded(v), true } -// normaliseDecoded converts map[any]any and []any leaves to types CEL can -// handle uniformly. JSON output is already compatible; YAML is the troubled -// case. +// normaliseDecoded converts map[any]any and []any leaves to types CEL can handle. func normaliseDecoded(v any) any { switch x := v.(type) { case map[any]any: @@ -579,10 +726,10 @@ func normaliseDecoded(v any) any { return v } -// BytesSize is a YAML-friendly byte count that accepts numbers or -// human-readable strings ("1MiB", "512KiB", "1MB", "1024b"). +// BytesSize is a YAML-friendly byte count with suffix support. type BytesSize int64 +// UnmarshalYAML implements Custom Unmarshal for BytesSize. func (b *BytesSize) UnmarshalYAML(n *yamlv3.Node) error { if n.Kind != yamlv3.ScalarNode { return fmt.Errorf("maxBodyBytes: expected scalar") diff --git a/internal/policy/policy_facts_test.go b/internal/policy/policy_facts_test.go index 4379374..e0cb5df 100644 --- a/internal/policy/policy_facts_test.go +++ b/internal/policy/policy_facts_test.go @@ -8,18 +8,18 @@ import ( ) // TestFactsURLIntegratedWithCEL boots a fake feed server, declares a -// `facts` URL source pointing at it, and exercises a CEL expression that -// parses the body and uses it inside inCIDR. +// `facts` URL source pointing at it, and exercises a CEL expression. func TestFactsURLIntegratedWithCEL(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("content-type", "application/json") - // Match the real ChatGPT / GCP-style shape: {prefixes:[{ipv4Prefix:"..."}]}. _, _ = w.Write([]byte(`{"creationTime":"now","prefixes":[{"ipv4Prefix":"10.0.0.0/8"},{"ipv4Prefix":"192.168.0.0/16"}]}`)) })) defer srv.Close() c, err := LoadBytes([]byte(` -defaults: { action: deny } +defaults: + extAuthz: + action: deny facts: - name: feed method: url @@ -29,12 +29,16 @@ facts: timeout: 2s groups: - name: g - action: allow + parameters: + engine: extAuthz + mode: firstMatch rules: - name: ok match: | inCIDR(request.remoteIp, parseJSON(facts.feed).prefixes.map(p, p.ipv4Prefix)) + validation: + action: allow `)) if err != nil { t.Fatalf("load: %v", err) @@ -55,7 +59,9 @@ groups: // TestFactsInlineValue exercises the value method with a literal list. func TestFactsInlineValue(t *testing.T) { c, err := LoadBytes([]byte(` -defaults: { action: deny } +defaults: + extAuthz: + action: deny facts: - name: cidrs method: value @@ -64,10 +70,14 @@ facts: - 192.168.0.0/16 groups: - name: g - action: allow + parameters: + engine: extAuthz + mode: firstMatch rules: - name: ok match: inCIDR(request.remoteIp, facts.cidrs) + validation: + action: allow `)) if err != nil { t.Fatalf("load: %v", err) diff --git a/internal/policy/policy_test.go b/internal/policy/policy_test.go index cbdafa5..0fb4236 100644 --- a/internal/policy/policy_test.go +++ b/internal/policy/policy_test.go @@ -3,6 +3,7 @@ package policy import ( "context" "net/http" + "strings" "testing" ) @@ -27,12 +28,19 @@ func ev(c *Config, r *Request) Decision { return c.Evaluate(context.Background() func TestDefaultDeny(t *testing.T) { c := mustLoad(t, ` -defaults: { action: deny } +defaults: + extAuthz: + action: deny groups: - name: noop + parameters: + engine: extAuthz + mode: firstMatch rules: - name: never match: "false" + validation: + action: allow `) d := ev(c, mkReq("GET", "x", "/", "", nil, "")) if d.Allowed || d.Rule != "" { @@ -40,82 +48,87 @@ groups: } } -func TestFirstMatchInheritance(t *testing.T) { +func TestFirstMatchExtAuthz(t *testing.T) { c := mustLoad(t, ` -defaults: { action: deny } +defaults: + extAuthz: + action: deny groups: - name: providers - mode: firstMatch - action: allow + parameters: + engine: extAuthz + mode: firstMatch rules: - name: a match: "request.host == 'a.example.com'" + validation: + action: allow - name: b match: "request.host == 'b.example.com'" + validation: + action: allow + - name: blocked + match: "request.host == 'blocked.example.com'" + validation: + action: deny `) + + // a matches -> allows if d := ev(c, mkReq("GET", "a.example.com", "/", "", nil, "")); !d.Allowed || d.Rule != "providers/a" { t.Fatalf("a: %+v", d) } + // b matches -> allows if d := ev(c, mkReq("GET", "b.example.com", "/", "", nil, "")); !d.Allowed || d.Rule != "providers/b" { t.Fatalf("b: %+v", d) } - if d := ev(c, mkReq("GET", "c.example.com", "/", "", nil, "")); d.Allowed { - t.Fatalf("c: %+v", d) - } -} - -func TestRuleActionOverridesGroup(t *testing.T) { - c := mustLoad(t, ` -defaults: { action: deny } -groups: - - name: providers - mode: firstMatch - action: allow - rules: - - name: blocked - action: deny - match: "size(request.body.json.redirect_uris) > 5" - - name: ok - match: "true" -`) - // Too many redirects -> rule "blocked" denies. - if d := ev(c, mkReq("POST", "x", "/", `{"redirect_uris":["a","b","c","d","e","f"]}`, map[string]string{"content-type": "application/json"}, "")); d.Allowed { + // blocked matches -> denies + if d := ev(c, mkReq("GET", "blocked.example.com", "/", "", nil, "")); d.Allowed || d.Rule != "providers/blocked" { t.Fatalf("blocked: %+v", d) } - // Few -> rule "ok" allows. - if d := ev(c, mkReq("POST", "x", "/", `{"redirect_uris":["a"]}`, map[string]string{"content-type": "application/json"}, "")); !d.Allowed || d.Rule != "providers/ok" { - t.Fatalf("ok: %+v", d) + // c does not match any rule -> falls through to defaults + if d := ev(c, mkReq("GET", "c.example.com", "/", "", nil, "")); d.Allowed || d.Rule != "" { + t.Fatalf("c: %+v", d) } } -func TestModeAll(t *testing.T) { +func TestMatchAllExtAuthz(t *testing.T) { c := mustLoad(t, ` -defaults: { action: deny } +defaults: + extAuthz: + action: deny groups: - name: admin - mode: all - action: allow + parameters: + engine: extAuthz + mode: matchAll match: "request.path.startsWith('/admin')" rules: - name: internal match: "inCIDR(request.remoteIp, ['10.0.0.0/8'])" + validation: + action: allow - name: group match: "request.header['x-user-groups'].contains('platform-admins')" - - name: nodebug - match: "!has('x-debug', request.headers)" + validation: + action: allow `) + + // All match -> allowed ok := ev(c, mkReq("GET", "x", "/admin/x", "", map[string]string{"x-user-groups": "platform-admins"}, "10.0.0.1")) if !ok.Allowed { t.Fatalf("all-pass: %+v", ok) } + + // First fails -> deny (canary checks that we do not let it pass if matchAll fails) miss := ev(c, mkReq("GET", "x", "/admin/x", "", nil, "10.0.0.1")) if miss.Allowed { - t.Fatalf("missing group: %+v", miss) + t.Fatalf("missing group should be denied in matchAll: %+v", miss) } - debug := ev(c, mkReq("GET", "x", "/admin/x", "", map[string]string{"x-user-groups": "platform-admins", "x-debug": "1"}, "10.0.0.1")) - if debug.Allowed { - t.Fatalf("debug present: %+v", debug) + if miss.Rule != "admin/group" { + t.Fatalf("expected rule to be group-fail, got %q", miss.Rule) } + + // Outside path -> falls through (group match guard) out := ev(c, mkReq("GET", "x", "/elsewhere", "", map[string]string{"x-user-groups": "platform-admins"}, "10.0.0.1")) if out.Allowed { t.Fatalf("out of scope: %+v", out) @@ -125,19 +138,334 @@ groups: } } +func TestGroupMatchGuard(t *testing.T) { + c := mustLoad(t, ` +defaults: + extAuthz: + action: deny +groups: + - name: skipGroup + parameters: + engine: extAuthz + mode: firstMatch + match: "request.path == '/special'" + rules: + - name: r1 + match: "true" + validation: + action: allow +`) + + // Matches guard -> allowed + d1 := ev(c, mkReq("GET", "x", "/special", "", nil, "")) + if !d1.Allowed || d1.Rule != "skipGroup/r1" { + t.Fatalf("expected group match: %+v", d1) + } + + // Fails guard -> skips group -> default deny + d2 := ev(c, mkReq("GET", "x", "/other", "", nil, "")) + if d2.Allowed || d2.Rule != "" { + t.Fatalf("expected skip: %+v", d2) + } +} + +func TestLoadErrors(t *testing.T) { + cases := []struct { + name string + src string + }{ + { + "engine missing", + ` +groups: + - name: g + parameters: + mode: firstMatch + rules: + - name: r + match: "true" + validation: { action: allow } +`, + }, + { + "engine invalid", + ` +groups: + - name: g + parameters: + engine: extAuthz2 + mode: firstMatch + rules: + - name: r + match: "true" + validation: { action: allow } +`, + }, + { + "mode invalid for extAuthz", + ` +groups: + - name: g + parameters: + engine: extAuthz + mode: applyAll + rules: + - name: r + match: "true" + validation: { action: allow } +`, + }, + { + "mode invalid for extProc", + ` +groups: + - name: g + parameters: + engine: extProc + mode: matchAll + phase: requestHeaders + rules: + - name: r + match: "true" + mutations: [] +`, + }, + { + "phase in extAuthz", + ` +groups: + - name: g + parameters: + engine: extAuthz + mode: firstMatch + phase: requestHeaders + rules: + - name: r + match: "true" + validation: { action: allow } +`, + }, + { + "phase missing in extProc", + ` +groups: + - name: g + parameters: + engine: extProc + mode: firstMatch + rules: + - name: r + match: "true" + mutations: [] +`, + }, + { + "extAuthz rule with mutations", + ` +groups: + - name: g + parameters: + engine: extAuthz + mode: firstMatch + rules: + - name: r + match: "true" + validation: { action: allow } + mutations: + - { op: removeHeader, name: foo } +`, + }, + { + "extProc rule with validation", + ` +groups: + - name: g + parameters: + engine: extProc + mode: firstMatch + phase: requestHeaders + rules: + - name: r + match: "true" + validation: { action: allow } + mutations: [] +`, + }, + { + "rule match missing", + ` +groups: + - name: g + parameters: + engine: extAuthz + mode: firstMatch + rules: + - name: r + validation: { action: allow } +`, + }, + { + "mutation unknown op", + ` +groups: + - name: g + parameters: + engine: extProc + mode: firstMatch + phase: requestHeaders + rules: + - name: r + match: "true" + mutations: + - { op: dance, name: foo } +`, + }, + { + "setStatus in request phase", + ` +groups: + - name: g + parameters: + engine: extProc + mode: firstMatch + phase: requestHeaders + rules: + - name: r + match: "true" + mutations: + - { op: setStatus, code: "200" } +`, + }, + { + "setBody in headers phase", + ` +groups: + - name: g + parameters: + engine: extProc + mode: firstMatch + phase: requestHeaders + rules: + - name: r + match: "true" + mutations: + - { op: setBody, value: "'foo'" } +`, + }, + { + "onBodyOverflow invalid", + ` +defaults: + extProc: + onBodyOverflow: explode +groups: + - name: g + parameters: + engine: extAuthz + mode: firstMatch + rules: + - name: r + match: "true" + validation: { action: allow } +`, + }, + { + "action invalid", + ` +groups: + - name: g + parameters: + engine: extAuthz + mode: firstMatch + rules: + - name: r + match: "true" + validation: { action: explode } +`, + }, + { + "matchAll with action deny", + ` +groups: + - name: g + parameters: + engine: extAuthz + mode: matchAll + rules: + - name: r + match: "true" + validation: { action: deny } +`, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := LoadBytes([]byte(tc.src)) + if err == nil { + t.Fatalf("expected error for case %q, but loaded successfully", tc.name) + } + }) + } +} + +func TestCompilationScope(t *testing.T) { + // response.status used in responseHeaders compiles OK + srcOK := ` +groups: + - name: g + parameters: + engine: extProc + mode: firstMatch + phase: responseHeaders + rules: + - name: r + match: "response.status == 200" + mutations: [] +` + if _, err := LoadBytes([]byte(srcOK)); err != nil { + t.Fatalf("expected compilation success: %v", err) + } + + // response.status used in requestHeaders fails compilation + srcFail := ` +groups: + - name: g + parameters: + engine: extProc + mode: firstMatch + phase: requestHeaders + rules: + - name: r + match: "response.status == 200" + mutations: [] +` + _, err := LoadBytes([]byte(srcFail)) + if err == nil { + t.Fatal("expected compilation failure due to response variable out of scope, but succeeded") + } + if !strings.Contains(err.Error(), "response") && !strings.Contains(err.Error(), "undeclared reference") { + t.Fatalf("expected error referencing scope or response variable, got: %v", err) + } +} + func TestDryRun(t *testing.T) { c := mustLoad(t, ` -defaults: { action: allow } +defaults: + extAuthz: + action: allow groups: - name: shadow - action: deny + parameters: + engine: extAuthz + mode: firstMatch rules: - name: would-deny dryRun: true match: "request.path.startsWith('/forbidden')" + validation: + action: deny `) - // Evaluate returns the deny verdict with DryRun=true; enforcement - // suppression is the httpserver layer's responsibility. d := ev(c, mkReq("GET", "x", "/forbidden/x", "", nil, "")) if d.Allowed { t.Fatalf("Evaluate must return real deny verdict for dryRun rule: %+v", d) @@ -145,7 +473,6 @@ groups: if !d.DryRun { t.Fatalf("DryRun flag must be set: %+v", d) } - // Non-matching path: no rule fires, falls through to defaults (allow). d2 := ev(c, mkReq("GET", "x", "/safe", "", nil, "")) if !d2.Allowed { t.Fatalf("non-matching path should reach defaults allow: %+v", d2) @@ -157,38 +484,53 @@ groups: func TestDefaultsDryRunField(t *testing.T) { cases := []struct { - name string - src string - want bool + name string + src string + want bool }{ {"dryRun true", ` defaults: - action: deny + extAuthz: + action: deny dryRun: true groups: - name: g + parameters: + engine: extAuthz + mode: firstMatch rules: - name: r match: "true" + validation: { action: allow } `, true}, {"dryRun false explicit", ` defaults: - action: deny + extAuthz: + action: deny dryRun: false groups: - name: g + parameters: + engine: extAuthz + mode: firstMatch rules: - name: r match: "true" + validation: { action: allow } `, false}, {"dryRun omitted defaults false", ` defaults: - action: deny + extAuthz: + action: deny groups: - name: g + parameters: + engine: extAuthz + mode: firstMatch rules: - name: r match: "true" + validation: { action: allow } `, false}, } for _, tc := range cases { @@ -203,18 +545,26 @@ groups: func TestBodyJSONAndYAML(t *testing.T) { c := mustLoad(t, ` -defaults: { action: deny } +defaults: + extAuthz: + action: deny groups: - name: json - action: allow + parameters: + engine: extAuthz + mode: firstMatch rules: - name: jsonAmount match: "request.body.jsonOk && request.body.json.amount > 100" + validation: { action: allow } - name: yamlGroup - action: allow + parameters: + engine: extAuthz + mode: firstMatch rules: - name: yamlKind match: "request.body.yamlOk && request.body.yaml.kind == 'Deployment'" + validation: { action: allow } `) if d := ev(c, mkReq("POST", "x", "/", `{"amount":500}`, nil, "")); !d.Allowed { t.Fatalf("json: %+v", d) @@ -234,7 +584,6 @@ func TestExampleConfigFlow(t *testing.T) { } headers := map[string]string{"content-type": "application/json"} - // Helper: build a DCR POST against the mcp realm on the public host. dcr := func(body, remoteIP string) *Request { return mkReq("POST", "auth.example-2.com", "/realms/mcp/clients-registrations", body, headers, remoteIP) @@ -247,45 +596,26 @@ func TestExampleConfigFlow(t *testing.T) { want bool wantRule string }{ - // Corporate hostname is always denied. { "corporate DCR -> deny", mkReq("POST", "keycloak.internal.example-1.com", "/realms/mcp/clients-registrations", `{"redirect_uris":["https://x"]}`, headers, "10.5.5.5"), false, "dcr-corporate-deny/block-corporate", }, - - // Internal CIDR. {"internal source allowed", dcr(mcpBody, "10.5.5.5"), true, "dcr-internal/from-internal-cidr"}, {"non-internal source not allowed by internal rule", dcr(mcpBody, "8.8.8.8"), false, ""}, - - // Anthropic / Claude. {"claude new /21", dcr(mcpBody, "160.79.108.42"), true, "dcr-claude/from-claude-cidr"}, {"claude legacy individual", dcr(mcpBody, "34.162.142.92"), true, "dcr-claude/from-claude-cidr"}, {"random IP not in claude", dcr(mcpBody, "9.9.9.9"), false, ""}, - - // OpenAI / ChatGPT. - // The ChatGPT rule's match expression includes a guard - // `facts.chatgptFeed != null && facts.chatgptFeed != ""`, so when - // the URL feed hasn't been fetched (the case here - no .Start) the - // whole group is skipped silently. End-to-end coverage of the URL - // fetch flow lives in policy_facts_test.go. {"chatgpt without feed populated falls through to defaults", dcr(mcpBody, "13.65.138.115"), false, ""}, - - // Mistral, declared as dryRun in the example. - // The group match guard (`facts.mistralFeed != null && ...`) is false - // because the feed is never started in unit tests, so the group is - // skipped entirely and defaults deny applies. {"mistral placeholder not matched", dcr(mcpBody, "1.2.3.4"), false, ""}, - - // Google Antigravity. { "antigravity canonical google host", dcr(`{"redirect_uris":["https://antigravity.google/oauth-callback"]}`, "203.0.113.5"), @@ -326,15 +656,11 @@ func TestExampleConfigFlow(t *testing.T) { dcr(`{"redirect_uris":["https://localhost:51234/cb","https://attacker.com/cb"]}`, "203.0.113.5"), false, "", }, - - // Master realm is never reachable through public hosts. { "master realm on public host -> deny", mkReq("GET", "auth.example-2.com", "/realms/master/account", "", nil, "8.8.8.8"), false, "block-master-realm-public/block", }, - - // Defaults catch-all. { "random request -> default deny", mkReq("GET", "random.example.com", "/whatever", "", nil, "8.8.8.8"), @@ -364,20 +690,212 @@ func TestBytesSizeUnits(t *testing.T) { for src, want := range cases { yaml := ` defaults: - action: allow - ` + src + ` + extAuthz: + action: allow + ` + src + ` groups: - name: dummy + parameters: + engine: extAuthz + mode: firstMatch rules: - name: any match: "true" + validation: { action: allow } ` c, err := LoadBytes([]byte(yaml)) if err != nil { t.Fatalf("%s: %v", src, err) } - if c.Defaults.MaxBodyBytes.Int64() != want { - t.Fatalf("%s: got %d want %d", src, c.Defaults.MaxBodyBytes.Int64(), want) + if c.Defaults.ExtAuthz.MaxBodyBytes.Int64() != want { + t.Fatalf("%s: got %d want %d", src, c.Defaults.ExtAuthz.MaxBodyBytes.Int64(), want) } } } + +func TestDirectResponse_ValidationErrors(t *testing.T) { + tests := []struct { + name string + yaml string + wantErrSub string + }{ + { + name: "directResponse in requestHeaders phase", + yaml: ` +groups: + - name: g1 + parameters: + engine: extProc + mode: firstMatch + phase: requestHeaders + rules: + - name: r1 + match: "true" + mutations: + - op: directResponse + status: 200 + headers: '{"a":"b"}' + body: '"body"' +`, + wantErrSub: "directResponse is only legal in responseHeaders|responseBody phases", + }, + { + name: "directResponse mixed with setHeader (exclusivity)", + yaml: ` +groups: + - name: g1 + parameters: + engine: extProc + mode: firstMatch + phase: responseHeaders + rules: + - name: r1 + match: "true" + mutations: + - op: directResponse + status: 200 + headers: '{"a":"b"}' + body: '"body"' + - op: setHeader + name: x-some + value: '"value"' +`, + wantErrSub: "directResponse must be the only mutation in the rule", + }, + { + name: "directResponse with name present", + yaml: ` +groups: + - name: g1 + parameters: + engine: extProc + mode: firstMatch + phase: responseHeaders + rules: + - name: r1 + match: "true" + mutations: + - op: directResponse + status: 200 + name: "forbidden-name" + headers: '{"a":"b"}' + body: '"body"' +`, + wantErrSub: "name must be empty", + }, + { + name: "directResponse with value present", + yaml: ` +groups: + - name: g1 + parameters: + engine: extProc + mode: firstMatch + phase: responseHeaders + rules: + - name: r1 + match: "true" + mutations: + - op: directResponse + status: 200 + value: "forbidden-value" + headers: '{"a":"b"}' + body: '"body"' +`, + wantErrSub: "value must be empty", + }, + { + name: "directResponse with code present", + yaml: ` +groups: + - name: g1 + parameters: + engine: extProc + mode: firstMatch + phase: responseHeaders + rules: + - name: r1 + match: "true" + mutations: + - op: directResponse + status: 200 + code: "forbidden-code" + headers: '{"a":"b"}' + body: '"body"' +`, + wantErrSub: "code must be empty", + }, + { + name: "directResponse with status 0", + yaml: ` +groups: + - name: g1 + parameters: + engine: extProc + mode: firstMatch + phase: responseHeaders + rules: + - name: r1 + match: "true" + mutations: + - op: directResponse + status: 0 + headers: '{"a":"b"}' + body: '"body"' +`, + wantErrSub: "status is required, must be a valid HTTP code", + }, + { + name: "directResponse with empty headers", + yaml: ` +groups: + - name: g1 + parameters: + engine: extProc + mode: firstMatch + phase: responseHeaders + rules: + - name: r1 + match: "true" + mutations: + - op: directResponse + status: 200 + headers: "" + body: '"body"' +`, + wantErrSub: "headers is required and cannot be empty", + }, + { + name: "directResponse with empty body", + yaml: ` +groups: + - name: g1 + parameters: + engine: extProc + mode: firstMatch + phase: responseHeaders + rules: + - name: r1 + match: "true" + mutations: + - op: directResponse + status: 200 + headers: '{"a":"b"}' + body: "" +`, + wantErrSub: "body is required and cannot be empty", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := LoadBytes([]byte(tt.yaml)) + if err == nil { + t.Fatalf("expected error containing %q, got nil", tt.wantErrSub) + } + if !strings.Contains(err.Error(), tt.wantErrSub) { + t.Fatalf("expected error containing %q, got: %v", tt.wantErrSub, err) + } + }) + } +}