Skip to content

Commit 9e7085b

Browse files
committed
docs(body-aware): clarify v1 scope per Gemini Code Assist feedback
Three medium-priority concerns from the review needed reconciliation between the design doc and the v1 implementation: 1. Pre-eval optimization with require_body_eval: doc described a flag but didn't say the header-side allow rule should still run first to short-circuit deny on header-only decisions. v1 doesn't ship the flag at all -- body callback always evaluates allow_body when it fires. Reframed as 'v1 vs v2': v2 lands the flag plus the header-phase short-circuit (saves CPU and buffering on rejected requests). Static-AST flag inference noted via the streaming evaluation proposal. 2. RequestContext lifecycle in host_allocator: doc proposed the snapshot map without saying anything about the deep-free / leak risk for the inner slices and json.Value tree. v1 doesn't ship per-context state; the snapshot is v2. Doc now recommends a per-context arena (lazy on first header callback, reset on proxy_on_done) so v2 starts from a shape that doesn't need manual deep-frees. 3. 'body: undefined' on cap exceedance: doc was wrong, undefined isn't valid JSON. v1's actual behaviour: body is set to JSON null when the body fails to parse, is empty, or was truncated by the host's max_body_bytes cap. body_raw always carries the raw (capped) bytes. Doc now states this explicitly and shows how Rego-style policies can branch on body_raw == '' vs body == null to distinguish 'no body' from 'non-JSON body'. Input shape section now splits v1 (body-only) from v2 (body plus request snapshot) so existing 'allow' rules are visibly unaffected.
1 parent fe45fc6 commit 9e7085b

1 file changed

Lines changed: 70 additions & 21 deletions

File tree

docs/proposals/body-aware-policies.md

Lines changed: 70 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,33 @@ cannot be evaluated in either callback alone.
2424

2525
## Goals
2626

27-
1. In `proxy_on_request_headers`, snapshot `:method`, `:path`,
28-
`:authority`, and a configurable subset of request headers into
29-
per-context state.
3027
1. Implement `proxy_on_request_body`:
3128
- Wait for `end_of_stream` (or buffer up to `max_body_bytes`).
3229
- Read the body via `proxy_get_buffer_bytes(BufferType.HttpRequestBody)`.
33-
- Build an `input` JSON containing snapshot + parsed body.
34-
- Run `evaluate` against the configured policy AST.
30+
- Build an `input` JSON containing the parsed body.
31+
- Evaluate the AST against a separate target rule `allow_body`.
3532
- On deny, call `proxy_send_local_response(403)` and return Pause;
3633
on allow, return Continue.
37-
1. Add an opt-in plugin config flag `require_body_eval: true` so hosts
38-
that don't need body inspection don't pay the buffering cost.
34+
35+
### v1 vs v2
36+
37+
**v1 (this PR)**: body callback always runs against `allow_body`
38+
when it fires, with body-only input (no request snapshot, no
39+
opt-in flag, no per-prefix optimization). Header-side `allow` keeps
40+
its existing flat input untouched.
41+
42+
**v2 (deferred)**:
43+
44+
- Per-context snapshot of `:method` / `:path` / `:authority` /
45+
selected headers in `proxy_on_request_headers`, surfaced to the
46+
body rule as `input.method` etc.
47+
- Opt-in plugin config flag `require_body_eval: true` so hosts
48+
that don't need body inspection skip buffering. When the flag
49+
is on, the header phase still evaluates `allow` and short-circuits
50+
on header-only deny decisions before the body fires (saves CPU
51+
and the buffering cost on rejected requests).
52+
- Static AST analysis (see `streaming-evaluation.md`) can flip the
53+
flag automatically when the policy references `input.body.*`.
3954

4055
## Non-goals
4156

@@ -48,7 +63,9 @@ cannot be evaluated in either callback alone.
4863

4964
## Design sketch
5065

51-
### Per-context state
66+
### Per-context state (v2 only)
67+
68+
The v2 snapshot would look like:
5269

5370
```zig
5471
const RequestContext = struct {
@@ -59,36 +76,68 @@ const RequestContext = struct {
5976
};
6077
```
6178

62-
A small `AutoHashMap(u32, *RequestContext)` keyed by `context_id` lives
63-
in `host_allocator`. Cleared on `proxy_on_done`.
79+
with a `AutoHashMap(u32, *RequestContext)` keyed by `context_id`. The
80+
naive design holds the map in `host_allocator`, which means every
81+
field carries a manual `defer free` and `proxy_on_done` has to deep-
82+
free the inner string slices and the parsed `json.Value` tree.
83+
84+
A cleaner approach (recommended, captured here so v2 starts from the
85+
right shape): give each context its **own arena**, allocated lazily
86+
on the first header callback and reset / freed in `proxy_on_done`.
87+
Saves the per-field free dance.
88+
89+
**v1 has no per-context state.** Header / body callbacks operate
90+
independently and the body rule only sees the body itself.
6491

6592
### Input shape
6693

94+
v1 (this PR) — body-only:
95+
6796
```json
6897
{
69-
"method": "POST",
70-
"path": "/orders",
98+
"body": { "amount": 250 },
99+
"body_raw": "{\"amount\":250}"
100+
}
101+
```
102+
103+
v2 (deferred) — body plus the request snapshot:
104+
105+
```json
106+
{
107+
"method": "POST",
108+
"path": "/orders",
71109
"headers": { "...": "..." },
72110
"body": { "amount": 250 },
73111
"body_raw": "{\"amount\":250}"
74112
}
75113
```
76114

77-
`body` is present iff the body parsed as JSON. `body_raw` is always
78-
present once body eval ran.
115+
`body` is set to JSON `null` (not `undefined` -- that is not a JSON
116+
value) when the body fails to parse as JSON, when the body is empty,
117+
or when the read was truncated by `max_body_bytes`. In every case
118+
`body_raw` carries whatever bytes the host returned (capped). Rego-
119+
style policies that want to distinguish "no body" from "non-JSON
120+
body" can branch on `body_raw == ""` vs `body == null`.
79121

80122
### Buffer limit
81123

82-
Configurable via plugin config: `max_body_bytes` (default 64 KiB). When
83-
exceeded, the policy sees `body: undefined` and `body_raw` truncated.
84-
Mirrors Envoy's own `max_request_bytes` posture.
124+
v1 hardcodes `max_body_bytes = 64 * 1024`. v2 will lift this into
125+
the plugin config alongside `require_body_eval`. When the host
126+
returns more than the cap, `proxy_get_buffer_bytes(start=0, max=cap)`
127+
already truncates on the host side -- v1 does not re-truncate, so
128+
`body_raw` length is always `<= cap` and `body` is `null` whenever
129+
the truncated bytes do not parse as a complete JSON document.
85130

86131
## API impact
87132

88-
- `proxy_on_request_body` returns `Action.Pause` until evaluation
89-
completes. Behavior change, but only when the host opts in via
90-
`require_body_eval: true`.
91-
- New AST refs become valid: `input.body.<path>`, `input.body_raw`.
133+
- `proxy_on_request_body` returns `Action.Pause` on deny only (sends
134+
403 first via `proxy_send_local_response`). Allow returns Continue.
135+
- New target rule name `allow_body` joins `allow` and `allow_response`.
136+
- New AST refs become valid under `allow_body`: `input.body.<path>`,
137+
`input.body_raw`.
138+
- Existing `allow` policies continue to work unchanged (request-side
139+
input shape stays flat with `input.method` / `input.path` /
140+
`input.headers`).
92141

93142
## Test plan
94143

0 commit comments

Comments
 (0)