Skip to content

Commit 6edfb71

Browse files
nnemirovskynnemirovsky
committed
docs(plans): add per-request HTTP policy evaluation plan
1 parent d01716e commit 6edfb71

1 file changed

Lines changed: 179 additions & 0 deletions

File tree

‎docs/plans/20260408-per-request-policy.md‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
# Per-Request Policy Evaluation for HTTP/HTTPS and QUIC/HTTP3
2+
3+
## Overview
4+
5+
Move policy evaluation from the SOCKS5 CONNECT level (per-TCP-connection) to the HTTP request level for HTTP/HTTPS and QUIC/HTTP3. Currently "Allow Once" allows all HTTP requests on a keep-alive connection because policy is only checked when the TCP connection is established.
6+
7+
After this change, "Allow Once" means one HTTP request. Subsequent requests on the same keep-alive connection re-trigger the approval flow. gRPC is covered automatically since it rides over HTTP/2 through the same goproxy handler.
8+
9+
**Out of scope**: WebSocket (per-upgrade is correct since it's a single session), SSH (per-connection is correct since channels are part of one session), IMAP/SMTP (per-connection is correct since it's one mailbox session). Per-message/per-command policy on these protocols would hit the broker's 5/min rate limit and break normal usage.
10+
11+
## Context
12+
13+
- Policy evaluation: `internal/proxy/server.go` (Allow method)
14+
- HTTP/HTTPS MITM: `internal/proxy/inject.go` (goproxy OnRequest handler, `injectCredentials()`)
15+
- QUIC/HTTP3: `internal/proxy/quic.go` (HTTP handler in `buildHandler()`)
16+
- Approval broker: `internal/channel/broker.go`
17+
- Policy engine: `internal/policy/engine.go` (`Evaluate()`)
18+
- Telegram messages: `internal/telegram/bot.go` (`FormatApprovalMessage()`)
19+
20+
## Development Approach
21+
22+
- **Testing approach**: Regular (code first, then tests)
23+
- Complete each task fully before moving to the next
24+
- CRITICAL: every task MUST include new/updated tests
25+
- CRITICAL: all tests must pass before starting next task
26+
27+
## Testing Strategy
28+
29+
- **Unit tests**: mock the broker and policy engine to test allow-once consumption, fast-path skip, and approval blocking
30+
- **Integration tests**: existing proxy tests in `internal/proxy/server_test.go` exercise the full SOCKS5 -> MITM -> upstream chain. Add test cases for allow-once with two sequential HTTP requests on the same connection.
31+
- **e2e tests**: add an e2e test that makes two HTTP requests to an ask-verdict destination on the same keep-alive connection and verifies the second request triggers a new approval
32+
33+
## What Goes Where
34+
35+
- **Implementation Steps**: code changes, tests, documentation
36+
- **Post-Completion**: manual deployment verification, performance measurement
37+
38+
## Solution Overview
39+
40+
Add a per-connection `RequestPolicyChecker` that HTTP handlers call before forwarding each request.
41+
42+
**Lifecycle**: one `RequestPolicyChecker` per SOCKS5 connection. Created in `Allow()`, stored in the request context, passed to the goproxy handler via `ProxyCtx.UserData`.
43+
44+
**Fast path**: if the SOCKS5 CONNECT evaluated to "allow" from an explicit rule match (not default verdict), per-request checks are skipped. This requires `EvaluateDetailed()` on the policy engine to distinguish rule-match from default.
45+
46+
**Allow-once flow**: when an "ask" verdict is approved once, the checker permits one HTTP request then marks the approval as consumed. The next request on the same connection re-triggers the ask flow via the broker.
47+
48+
**Always-allow/deny**: these persist rules to the store and recompile the engine. Subsequent connections (and requests) are handled by the engine directly. No per-request tracking needed.
49+
50+
## Technical Details
51+
52+
**RequestPolicyChecker** (per-SOCKS5-connection):
53+
- `CheckAndConsume(dest, port) -> (Verdict, error)`: evaluates policy, triggers broker if "ask", consumes allow-once approval, returns final verdict
54+
- Internal state: `allowedOnce map[string]bool` keyed on `dest:port`, set to true on first allow-once approval, deleted after consumption
55+
- Thread-safe (multiple HTTP requests can arrive concurrently on HTTP/2)
56+
57+
**EvaluateDetailed** on policy.Engine:
58+
- Returns `(Verdict, MatchSource)` where MatchSource is `RuleMatch` or `DefaultVerdict`
59+
- Allows the fast path to distinguish "this host is explicitly allowed" from "default verdict is allow"
60+
61+
**goproxy plumbing**:
62+
- `ProxyCtx.UserData` currently carries pin ID (string). Change to a struct `proxyConnState{pinID string, checker *RequestPolicyChecker}`
63+
- The SOCKS5 dial function (`server.go dial()`) sets UserData when routing through the injector
64+
65+
## Progress Tracking
66+
67+
- Mark completed items with `[x]` immediately when done
68+
- Add newly discovered tasks with + prefix
69+
- Document issues/blockers with ! prefix
70+
- Update plan if implementation deviates from original scope
71+
72+
## Implementation Steps
73+
74+
### Task 1: Add EvaluateDetailed to policy engine
75+
76+
**Files:**
77+
- Modify: `internal/policy/engine.go`
78+
- Modify: `internal/policy/engine_test.go`
79+
80+
- [ ] Add `MatchSource` type (`RuleMatch`, `DefaultVerdict`)
81+
- [ ] Add `EvaluateDetailed(dest, port) (Verdict, MatchSource)` method
82+
- [ ] Refactor existing `Evaluate()` to call `EvaluateDetailed()` internally
83+
- [ ] Write tests for EvaluateDetailed: explicit rule returns RuleMatch
84+
- [ ] Write tests for EvaluateDetailed: default verdict returns DefaultVerdict
85+
- [ ] Run tests
86+
87+
### Task 2: Add RequestPolicyChecker
88+
89+
**Files:**
90+
- Create: `internal/proxy/request_policy.go`
91+
- Create: `internal/proxy/request_policy_test.go`
92+
93+
- [ ] Create `RequestPolicyChecker` struct with `allowedOnce` map and mutex
94+
- [ ] Implement `CheckAndConsume(dest, port) (Verdict, error)` that checks engine, triggers broker if ask, tracks allow-once
95+
- [ ] On allow-once response: permit current request, mark `(dest, port)` as consumed
96+
- [ ] On subsequent check for consumed `(dest, port)`: re-trigger ask flow
97+
- [ ] On always-allow response: permit without tracking (engine handles persistence)
98+
- [ ] Write tests: allow-once consumed after one call
99+
- [ ] Write tests: second call to consumed dest re-triggers ask
100+
- [ ] Write tests: always-allow not consumed
101+
- [ ] Write tests: deny returns immediately
102+
- [ ] Run tests
103+
104+
### Task 3: Wire RequestPolicyChecker into SOCKS5 context
105+
106+
**Files:**
107+
- Modify: `internal/proxy/server.go`
108+
- Modify: `internal/proxy/inject.go`
109+
110+
- [ ] Add context key `ctxKeyPerRequestPolicy`
111+
- [ ] In `Allow()`, use `EvaluateDetailed()`. If verdict is allow + RuleMatch, set context flag "skip per-request"
112+
- [ ] In `Allow()`, for ask-approved connections, create `RequestPolicyChecker` and store in context
113+
- [ ] For SNI-deferred connections, create checker in `sniPolicyCheck()`
114+
- [ ] Change `ProxyCtx.UserData` from string to `proxyConnState` struct carrying pin ID and checker
115+
- [ ] Update `dial()` to pass checker via UserData
116+
- [ ] Write tests for context propagation
117+
- [ ] Run tests
118+
119+
### Task 4: Per-request policy in HTTP/HTTPS MITM
120+
121+
**Files:**
122+
- Modify: `internal/proxy/inject.go`
123+
124+
- [ ] In `injectCredentials()`, extract checker from `ProxyCtx.UserData`
125+
- [ ] If checker is nil (explicit allow, no per-request check), proceed as before
126+
- [ ] If checker present, call `CheckAndConsume(host, port)` before credential injection
127+
- [ ] If verdict is deny/timeout, return `goproxy.NewResponse` with 403
128+
- [ ] If verdict is allow (from ask approval), proceed with injection
129+
- [ ] Note: gRPC is automatically covered (same handler, uses HTTP/2 path in goproxy)
130+
- [ ] Write tests: allow-once blocks second request on same connection
131+
- [ ] Write tests: explicit allow skips per-request check
132+
- [ ] Write tests: gRPC request goes through per-request check
133+
- [ ] Run tests
134+
135+
### Task 5: Per-request policy in QUIC/HTTP3
136+
137+
**Files:**
138+
- Modify: `internal/proxy/quic.go`
139+
140+
- [ ] In `buildHandler()` HTTP handler, add checker from connection state
141+
- [ ] Same logic as HTTP/HTTPS: check before injection, deny with 403, consume allow-once
142+
- [ ] Write tests for QUIC per-request policy
143+
- [ ] Run tests
144+
145+
### Task 6: Update Telegram approval messages with request context
146+
147+
**Files:**
148+
- Modify: `internal/channel/channel.go`
149+
- Modify: `internal/telegram/bot.go`
150+
151+
- [ ] Add optional `Method` and `Path` fields to `ApprovalRequest`
152+
- [ ] Update `FormatApprovalMessage()`: show "GET https://example.com/path" for per-request approvals
153+
- [ ] Add "(per-request)" label to distinguish from connection-level approvals
154+
- [ ] Write tests for updated message formatting
155+
- [ ] Run tests
156+
157+
### Task 7: Verify acceptance criteria
158+
159+
- [ ] Verify "Allow Once" blocks second HTTP request to same host on same connection
160+
- [ ] Verify "Always Allow" allows all requests (rule persisted, engine handles it)
161+
- [ ] Verify explicit allow rules skip per-request checks (no performance regression)
162+
- [ ] Verify per-request policy works for QUIC/HTTP3
163+
- [ ] Verify gRPC requests trigger per-request checks
164+
- [ ] Run full test suite: `go test ./... -v -timeout 30s`
165+
- [ ] Verify test coverage meets 70% threshold
166+
167+
### Task 8: [Final] Update documentation
168+
169+
- [ ] Update CLAUDE.md: document per-request policy for HTTP/HTTPS/QUIC, note that WebSocket/SSH/IMAP stay connection-level
170+
- [ ] Update README.md protocol table
171+
- [ ] Move this plan to `docs/plans/completed/`
172+
173+
## Post-Completion
174+
175+
**Manual verification:**
176+
- Deploy to knuth and test "Allow Once" with HTTP keep-alive (fetch same URL twice)
177+
- Verify approval messages show HTTP method and path
178+
- Verify WebSocket connections (MCP gateway) still work without per-message approval
179+
- Measure latency impact of per-request policy checks under normal usage

0 commit comments

Comments
 (0)