Skip to content

Commit 76e0bf7

Browse files
committed
docs(response): 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. Replacement contract: doc claimed structured replacement (status / body / headers from the rule's value). v1 actually returns bool from the evaluator and does a fixed 503 on deny. The structured replacement requires the evaluator to surface json.Value plus a discriminator and is deferred to a follow-up PR. Doc now splits 'v1' (fixed 503) from 'deferred' (structured). 2. 503 vs 403 asymmetry: doc didn't say which deny code response-side uses, request-side already returns 403. v1 uses 503 on the response side (upstream being replaced) vs 403 on the request side (request rejected before upstream). Doc now states this explicitly as intentional. 3. Backward compat for nested input shape: doc showed '{request, response}' nested input which would have broken existing input.method / input.path refs. v1 keeps the request-side input flat (unchanged) and adds 'input.response.*' ONLY for the allow_response target rule. input.request.* is reserved for the v2 post-snapshot picture. Doc now shows v1 vs v2 input shapes separately and clarifies 'allow' policies are untouched. Per-context state section also corrected: v1 does not snapshot request fields. The snapshot lives behind body-aware-policies.md (PR #6) and shows up here only when that lands.
1 parent 44817df commit 76e0bf7

1 file changed

Lines changed: 48 additions & 15 deletions

File tree

docs/proposals/response-side-policies.md

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ Use cases:
2727
1. Implement `proxy_on_response_headers`:
2828
- Build an input shape reflecting response status and headers.
2929
- Evaluate the AST against the response target rule.
30-
- On deny, replace the response (status + body + selected headers).
30+
- On deny, replace the response with a fixed status (see
31+
§Replacement contract for v1 behaviour and the deferred
32+
structured replacement).
3133
- On allow, fall through.
3234
1. Reuse the existing AST machinery. No new node types are needed for
3335
v1; only a new target rule and a new input shape.
@@ -46,9 +48,9 @@ Use cases:
4648

4749
### Per-context state
4850

49-
Borrow the snapshot from `body-aware-policies.md`: a per-context store
50-
that already holds request `:method` and `:path` so the response rule
51-
can reason about request → response pairs.
51+
Eventually we want a per-context store that already holds request
52+
`:method` / `:path` so the response rule can reason about
53+
request → response pairs (sketched in `body-aware-policies.md`):
5254

5355
```zig
5456
const ResponseContext = struct {
@@ -58,26 +60,51 @@ const ResponseContext = struct {
5860
};
5961
```
6062

63+
**v1 does not implement the snapshot.** The response rule sees only
64+
the response side. `input.request.*` becomes valid once the
65+
per-context plumbing lands; until then, body / response policies
66+
that need request context have to encode it via the host.
67+
6168
### Input shape
6269

70+
v1 (this PR) ships **only** the response subtree. The request-side
71+
`allow` rule keeps its existing flat shape (`input.method`,
72+
`input.path`, `input.headers`). They are disjoint inputs because
73+
they target disjoint rules.
74+
6375
```json
76+
// allow_response input (this PR)
6477
{
65-
"request": {
66-
"method": "GET",
67-
"path": "/admin/users",
68-
"headers": { "...": "..." }
69-
},
7078
"response": {
7179
"status": 500,
7280
"headers": { "server": "nginx/1.27", "...": "..." }
7381
}
7482
}
7583
```
7684

85+
The wider shape with both `input.request.*` and `input.response.*`
86+
visible together is the post-snapshot v2 picture and is not exposed
87+
yet:
88+
89+
```json
90+
// v2 (deferred): both subtrees once the snapshot lands
91+
{
92+
"request": { "method": "GET", "path": "/admin/users", "headers": {} },
93+
"response": { "status": 500, "headers": {} }
94+
}
95+
```
96+
7797
### Replacement contract
7898

79-
When the response rule denies, zopa calls `proxy_send_local_response`
80-
with parameters drawn from the rule's `value`:
99+
**v1**: rule denies (returns `false` or `nil`) → zopa calls
100+
`proxy_send_local_response(503, ...)` with empty body / no extra
101+
headers. Allow falls through unchanged. The deny status code is
102+
**503** here (the upstream response is being replaced) vs **403**
103+
on the request-side `allow` path (the request is being rejected
104+
before it reaches upstream). The asymmetry is intentional.
105+
106+
**Deferred**: rule returns a structured value carrying status / body
107+
/ headers, e.g.
81108

82109
```json
83110
{
@@ -90,14 +117,20 @@ with parameters drawn from the rule's `value`:
90117
}
91118
```
92119

93-
A bare boolean `false` keeps current behavior (replace with a static
94-
`503` and an empty body).
120+
This needs the evaluator to surface a `json.Value` (not a `bool`)
121+
to the proxy-wasm shim, plus a discriminator so the shim can tell
122+
"the policy returned a structured replacement" from "the policy
123+
returned a non-boolean truthy value (= allow)". Both changes are
124+
non-trivial and deferred to a follow-up PR.
95125

96126
## API impact
97127

98128
- New target rule name `allow_response` joins the existing `allow`.
99-
- Existing `allow` policies continue to work unchanged.
100-
- New AST refs become valid: `input.request.*`, `input.response.*`.
129+
- Existing `allow` policies continue to work unchanged: same input
130+
shape (`input.method` / `input.path` / `input.headers`), same
131+
return contract, same 403 deny status.
132+
- v1 adds the `input.response.*` ref namespace under `allow_response`.
133+
`input.request.*` is reserved for v2 once per-context state lands.
101134

102135
## Test plan
103136

0 commit comments

Comments
 (0)