Repository navigation
Fix: Refuse static-inject credentials over plain HTTP (403 plaintext_credential) - #1537
Conversation
…credential) A guest can send an absolute-form GET http://host/... through the forward proxy, and static-inject never read pctx.Scheme — a credential resolved for that host would be injected onto a request that crosses the network in cleartext (#1481, MOCA MI1 §8.5 item 3). The gate runs in the injector, after the resolver succeeds and before any header write, so the refusal names the credential rather than the scheme: plain HTTP to a host without a credential keeps its normal fail-closed treatment, and an empty scheme (listener couldn't tell) is not guessed to be plaintext. The check deliberately lives here rather than in a pre-pipeline gate or as a blanket rule for credential-writing plugins — only the resolver knows whether the host has a credential, and token-exchange injects onto plaintext in-cluster calls by design. The denial is 403 with code plaintext_credential, the MOCA egress contract's identifier verbatim rather than the usual static-inject.<kebab> form, so the conformance suite Cortex shares with moca-egress matches it without a translation table. It also records a deny invocation so the refusal is attributable in the session timeline. Plugin tests pin the contract case, its negative space (no credential means 401 unresolved-key, not 403), https and unknown-scheme pass-through, and the recorded invocation; a forward-proxy end-to-end test pins that the upstream is never dialed and the client sees the 403 with the code. Closes #1481 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Mariusz Sabath <mrsabath@gmail.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 3 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review follow-ups on the #1481 gate: - The extproc leg was verified only by reading the source: the plugin tests cover the gate unit-level and the forwardproxy test pins the proxy-sidecar leg, but nothing would fail if extproc's :scheme read regressed, silently stopping the envoy-sidecar shape of the gate. Two tests now drive Process with Envoy's own message wire — :scheme http to a credentialed host refuses 403 plaintext_credential, and :scheme https still injects via the SetHeaders mutation. - The deny invocation's Details labelled the resolver key "host", which is pctx.Host (host:port) under key_by: host but the configured static key — not a host — under key_by: static. Renamed to "key". - docs: the 403 fires only after the bearer and placeholder checks pass (a plaintext request failing one of those keeps its 401; either way nothing is sent), and a new paragraph names the redirect boundary — the gate judges the arrival scheme, a WritesDestination plugin can Redirect to an http target after the pipeline has run, and the inference-router's credential drop/overwrite is what covers that path. Signed-off-by: Mariusz Sabath <mrsabath@gmail.com> Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
huang195
left a comment
huang195
left a comment
There was a problem hiding this comment.
Summary
The gate is right for the forward proxy. It is wrong for inbound use: on the reverse proxy, pctx.Scheme is the scheme the client used to reach Cortex, not the scheme of the hop the credential goes out on. So an inbound static-inject behind a plaintext listener now refuses every request, even when the backend is https. That is MOCA's own AB1 shape. One must-fix, one suggestion, one nit.
Author: mrsabath (MEMBER — maintainer)
Areas reviewed: Go (plugin, extproc/forwardproxy/reverseproxy listeners), Docs
Agent/IDE config (.claude/.vscode): none
Commits: 2 commits, all signed-off: yes
CI status: passing
The PR's own tests pass locally (GOWORK=off go test ./plugins/staticinject/ ./listener/extproc/ ./listener/forwardproxy/ -run Plaintext), and gofmt is clean.
| // repo's usual `static-inject.<kebab>` form, so the egress conformance | ||
| // suite Cortex and moca-egress share matches it without a translation | ||
| // table — the same reasoning that freezes `x-authbridge-*` wire names. | ||
| if strings.EqualFold(pctx.Scheme, "http") { |
There was a problem hiding this comment.
must-fix: the gate also fires on inbound, where Scheme describes the wrong hop.
The reverse proxy sets Scheme: requestScheme(r) (reverseproxy/server.go:763), which is http whenever the client→Cortex connection is not TLS. The credential, though, leaves on the Cortex→backend hop, and that hop's scheme comes from reverse_proxy_backend.
This is a real deployment, not a hypothetical. MOCA's AB1 (rossoctl/moca deploy/knative/authbridge/ab1-deployment.yaml) runs static-inject in the inbound pipeline, with reverse_proxy_addr: ":8080" (plaintext) in front of reverse_proxy_backend: "https://api.anthropic.com". I checked it with a scratch test in core/listener/reverseproxy: NewServer with an httptest.NewTLSServer backend, a plaintext front, and that AB1 static-inject config (key_by: static, inject_header: x-api-key). The result is status=403 upstreamHits=0. Every LLM call through AB1 would be refused once it picks up this image.
The smallest fix is to judge only outbound requests, the case #1481 is about:
if pctx.Direction == pipeline.Outbound && strings.EqualFold(pctx.Scheme, "http") {That touches the tests too. Direction's zero value is Inbound, so TestPlaintextGateNamesTheCredentialNotTheScheme, TestPlaintextGateAllowsHTTPS and TestPlaintextGateIgnoresUnknownScheme need Direction: pipeline.Outbound (only TestDenyPlaintextCredential sets it today). Also worth adding one case: inbound + Scheme: "http" still injects.
If you want inbound covered as well, the reverse proxy would need to expose the backend scheme. That is a larger change, and #1481 does not ask for it.
There was a problem hiding this comment.
Fixed in a11bdf1 — and thank you for finding this one, it would have taken down
every LLM call through AB1.
I reproduced it before changing anything, with the real AB1 config fetched from moca
(deploy/knative/authbridge/ab1-deployment.yaml: inbound pipeline, reverse_proxy_addr: ":8080", reverse_proxy_backend: "https://api.anthropic.com", key_by: static,
inject_header: x-api-key). Same result you got: status=403 upstreamHits=0. After the
fix: status=200, backend sees x-api-key="sk-real-secret".
Your diagnosis of why is the part I want to confirm back, because it is the reason the
fix is a direction check and not something cleverer: pctx.Scheme on the reverse proxy is
requestScheme(r) (server.go:797), the client→Cortex hop. The credential's hop is
Cortex→backend, whose scheme lives in reverse_proxy_backend and is not reachable from
the plugin. So there is no way to answer the question correctly on inbound today — only
to stop asking it there.
Applied your suggested one-liner verbatim:
if pctx.Direction == pipeline.Outbound && strings.EqualFold(pctx.Scheme, "http") {On the tests, you were right about all three, and the consequence is slightly worse than
"they need updating": because Direction's zero value is Inbound, after the fix those
three would have passed for the wrong reason — on the direction rather than on the
scheme they claim to test. TestPlaintextGateNamesTheCredentialNotTheScheme especially,
since its whole point is which refusal wins. All three now set Direction: pipeline.Outbound, with a comment saying why on that one.
I added the inbound case you asked for (TestPlaintextGateDoesNotFireOnInbound), plus one
more that I think is the one that actually protects this: a listener-level test in
core/listener/reverseproxy driving a plaintext front and a real TLS backend
(TestStaticInject_InboundPlaintextFrontHTTPSBackend_StillInjects). The staticinject unit
tests build a pipeline.Context by hand, so a wrong-hop Scheme can only ever appear
through a real listener — which is exactly why the existing suite missed this. It is my
reproduction promoted to a regression test, and it fails against the pre-fix code.
I also documented the inbound boundary in docs/static-inject-plugin.md rather than
leaving it as a code comment, since an operator running static-inject inbound now owns
their front listener's transport and should be told so.
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com
| conformance suite Cortex shares with `moca-egress` matches it without | ||
| a translation table. | ||
|
|
||
| The refusal names the credential, not the scheme: plain HTTP to a host |
There was a problem hiding this comment.
suggestion: say who this breaks, and coordinate with MOCA. MOCA's AB2 smoke (deploy/knative/leaf-smoke.sh:174, and the README's allow-path example) sends curl -s http://echo-target/ -H "Authorization: Bearer $ECHO_CRED" through the forward proxy to a host with a credential. That is exactly the request this PR now refuses. It is the intended contract (MI1 §8.5), and AB2 is pinned to main-9c131ee, so nothing breaks today. But the next image bump fails the H2 allow-path check unless echo-target moves to TLS, or the smoke expects 403. A line in the PR body (or a linked moca change) would keep that from surprising someone.
There was a problem hiding this comment.
Done in a11bdf1. I verified the AB2 claim before writing it up — leaf-smoke.sh:174
does send curl -s http://echo-target/ -H "Authorization: Bearer $ECHO_CRED" through the
forward proxy to a credentialed host, so that is squarely what this gate refuses, and
your read of the timing is right: pinned image today, breaks on the next bump.
Rather than a line in the PR body (which disappears once merged), I put it in
docs/static-inject-plugin.md as a short "Deployments this refuses" table — forward-proxy
http:// with a credential is now 403, without one is unchanged, inbound is unchanged
— followed by the AB2 note naming the file, the claim, and the two ways out (echo-target
to TLS, or the smoke expecting 403). That way it is findable by whoever hits it rather
than only by whoever read this PR.
I have not opened the moca-side change; say the word and I will, or if you would rather it
go with whatever bumps the AB2 image that is probably the better pairing anyway.
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com
| // A credential that is about to be injected must never cross the network | ||
| // in cleartext (#1481, MOCA MI1 §8.5: 403 plaintext_credential). pctx.Scheme | ||
| // is "http" exactly for a request the listener knows is plaintext — an | ||
| // absolute-form http:// URL through the forward proxy, or Envoy's :scheme |
There was a problem hiding this comment.
nit: "or Envoy's :scheme on the ext_proc path": the envoy profile does not include staticinject (scripts/profile-tags/profiles.go:79), so no shipped envoy-sidecar image runs this gate. extproc/plaintextgate_test.go pins wiring that is fine to have, but the comment reads as if the envoy shape is covered in production. Consider "would be, if the envoy profile shipped it".
There was a problem hiding this comment.
Fixed in a11bdf1. Confirmed first: staticinject's profile list is
{ProfileFull, ProfileFullCpex, ProfileLite} (scripts/profile-tags/profiles.go) with no
ProfileEnvoy, so you are right that no shipped envoy-sidecar image runs this gate and the
comment was describing a path that does not exist in production.
The comment now reads "would be, if the envoy profile shipped it" and names
extproc/plaintextgate_test.go as pinning the wiring against that changing — so the test
keeps its purpose without the comment implying the envoy shape is covered today.
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com
The plaintext gate judged the wrong hop on inbound requests, and that broke a real deployment rather than a hypothetical one. On the reverse proxy `pctx.Scheme` is `requestScheme(r)` (`reverseproxy/server.go:797`) — the scheme the CLIENT used to reach Cortex, `http` for any non-TLS inbound listener. The credential does not travel on that hop. It leaves on the Cortex→backend hop, whose scheme comes from `reverse_proxy_backend` and is invisible from the plugin. So refusing an inbound request because its front was plaintext protects nothing and blocks everything. MOCA's AB1 is exactly that shape: static-inject in the INBOUND pipeline, `reverse_proxy_addr: ":8080"` plaintext, `reverse_proxy_backend: "https://api.anthropic.com"` (moca `deploy/knative/authbridge/ab1-deployment.yaml`, which I fetched and confirmed). I reproduced the breakage end to end before fixing: status 403, zero upstream hits, every LLM call through AB1 refused once it picked up this image. The gate now requires `pctx.Direction == pipeline.Outbound`, which is the case #1481 is about. Covering the inbound hop properly needs the reverse proxy to expose its backend scheme — a larger change this issue does not ask for, and the code and docs now say so rather than leaving it implied. Tests. Three existing gate tests relied on `Direction`'s zero value, which is Inbound — so after this change they would have passed for the wrong reason, on the direction rather than on the scheme. All three now set `Direction: pipeline.Outbound` explicitly, with a comment on the one where it is load-bearing. Two new tests: - `TestPlaintextGateDoesNotFireOnInbound` — the unit case. - `TestStaticInject_InboundPlaintextFrontHTTPSBackend_StillInjects` (`core/listener/reverseproxy`) — the end-to-end case, and the one that matters: the staticinject unit tests build a `pipeline.Context` by hand, so a wrong-hop `Scheme` can only appear through a real listener. This is my reproduction promoted to a regression test. Both fail against the code before the fix. Also from the review: - **Docs now name who this breaks.** MOCA's AB2 leaf smoke (`deploy/knative/leaf-smoke.sh`, the `H2-inject` claim) sends `curl -s http://echo-target/ -H "Authorization: Bearer $ECHO_CRED"` through the forward proxy to a credentialed host — refused by this gate. AB2 is pinned to an image predating it, so nothing breaks today, but the next bump fails that check unless echo-target moves to TLS or the smoke expects the 403. Added as a table of which shapes this refuses, plus the AB2 note. - **The envoy comment no longer overclaims.** `staticinject` is in `{full, fullcpex, lite}` and not `ProfileEnvoy` (`scripts/profile-tags/profiles.go`), so no shipped envoy-sidecar image runs this gate; the comment says "would be, if the envoy profile shipped it" and names the test that pins the wiring. Verified: full core suite, 72 packages, 0 failures; go vet and gofmt clean. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Mariusz Sabath <mrsabath@gmail.com>
|
All three review findings addressed in a11bdf1, and the branch is merged up to main On the conflictsThere weren't any — The must-fix@huang195's diagnosis was exactly right, and I reproduced it with the real AB1 config The gate now requires Two things worth flagging beyond what was asked: The three existing tests would have passed for the wrong reason. A unit test could not have caught this. The staticinject tests build a The suggestion and the nit
Verification
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com |
huang195
left a comment
huang195
left a comment
There was a problem hiding this comment.
Summary
Round 2: all three round-1 threads are fixed in a11bdf1. The direction check is right. I mutated it back to scheme-only and both new tests fail (TestPlaintextGateDoesNotFireOnInbound, TestStaticInject_InboundPlaintextFrontHTTPSBackend_StillInjects), so the AB1 regression is pinned at the listener level. One suggestion on the new inbound prose, which names the wrong hop as the operator's job, plus two nits.
On the moca-side change: pairing it with the AB2 image bump, as you suggested, sounds right to me.
Author: mrsabath (MEMBER — maintainer)
Areas reviewed: Go (staticinject plugin, reverseproxy/forwardproxy listeners), Docs
Agent/IDE config (.claude/.vscode): none
Commits: 4 commits (incl. a merge of main), all signed-off: yes
CI status: passing
At a11bdf1 I ran GOWORK=off go test ./plugins/staticinject/ ./listener/{extproc,forwardproxy,reverseproxy}/, go vet and gofmt -l, and all were clean.
| Covering the inbound hop properly would mean teaching the reverse | ||
| proxy to expose its backend scheme; that is not implemented. Until it | ||
| is, an operator running static-inject inbound is responsible for the | ||
| front listener's own transport (`mtls:`, or a trusted network), the |
There was a problem hiding this comment.
suggestion: this names the wrong hop as the operator's job. The paragraph above explains that the credential doesn't travel on the front hop. So the uncovered inbound risk isn't the front listener's transport. It is a reverse_proxy_backend of http://…: an inbound static-inject in front of a plaintext backend still injects the real key, which then crosses that hop in cleartext. mtls: on the front listener doesn't change that.
Something like: "Until it is, an operator running static-inject inbound is responsible for pointing reverse_proxy_backend at an https:// backend (or one on a trusted network). Cortex does not refuse an http:// backend." The code comment at plugin.go:247 ("an inbound plaintext hop is the operator's own listener") has the same slip.
There was a problem hiding this comment.
Fixed in 805529b — you're right, and the slip is worse than a wording
problem: the sentence contradicted the paragraph directly above it. That
paragraph exists to say the credential does not travel on the front hop, and
then the next one told the operator to go secure the front hop. Anyone acting
on it would have put mtls: on the listener, concluded they were covered, and
still been shipping the real key in cleartext to an http:// backend.
Took your wording, with the explicit non-guarantee attached:
Until it is, an operator running static-inject inbound is responsible for
pointingreverse_proxy_backendat anhttps://backend (or one on a
trusted network). That is the hop the credential actually travels on: an
inbound static-inject in front of a plaintext backend still injects the real
key, which then crosses that hop in cleartext. Cortex does not refuse an
http://backend, andmtls:on the front listener does not change it.
I added the "Cortex does not refuse an http:// backend" line because the
gate's existence invites the opposite assumption — a reader who has just
learned that Cortex 403s plaintext egress could reasonably expect it to do the
same inbound. Worth stating that it does not.
And you were right that plugin.go had the same slip; I'd written the two in
one sitting. That comment now names the http:// backend as the uncovered
risk and says the front listener's transport is not what matters, since the
credential never travels on it.
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com
|
|
||
| | Shape | Effect | | ||
| |---|---| | ||
| | Forward-proxy egress, `http://` target **with** a credential | **Now 403.** The request must move to `https://`, or the host must stop having a credential. | |
There was a problem hiding this comment.
nit: there's one more shape this refuses that the table doesn't list. Under mtls: mode: strict, the forward proxy dials every upstream through mtlsDialer (forwardproxy/server.go:235), so an http:// target is actually TLS-wrapped on the wire. The gate still sees Scheme: "http" and returns 403. That's fail-closed and probably rare (static-inject plus strict mesh mTLS), but a row saying so would save someone a confusing 403.
There was a problem hiding this comment.
Added in 805529b. Confirmed the mechanism before writing the row: when
MTLSOptions is non-nil the transport's DialContext is replaced wholesale
with mtlsDialer(...).DialContext (forwardproxy/server.go), and that dialer
is TLS-or-fail per its own contract — plain TCP, then a handshake with the
local SVID, no per-connection fallback. It keys off nothing in the request, so
the target's scheme never enters into it. An http:// target is genuinely TLS
on the wire, and the gate — which reads pctx.Scheme, not the dialer — still
returns 403.
New row:
| Forward-proxy egress,
http://target with a credential, undermtls: mode: strict| Now 403, even though the hop is TLS on the wire. Strict mode dials every upstream TLS-or-fail (mtlsDialer), but the gate reads the request's scheme, not the dialer, so it cannot see that. Fail-closed, and rare (static-inject plus strict mesh mTLS); move the target tohttps://to inject. |
I left the behaviour alone deliberately, and I'd rather not "fix" it: teaching
the gate to consult the dialer would make the refusal depend on mesh config,
so the same request would be allowed or refused depending on a setting that
lives nowhere near the credential. Fail-closed plus a documented row is the
better trade. Agreed it's rare — it needs static-inject and strict mesh mTLS
on the same egress path — but as you say, that's precisely the person who'd
have no idea why they got a 403.
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com
| // | ||
| // OUTBOUND ONLY, and the direction is load-bearing rather than a | ||
| // narrowing for caution's sake. On the reverse proxy pctx.Scheme is | ||
| // requestScheme(r) (reverseproxy/server.go:797) — the scheme the CLIENT |
There was a problem hiding this comment.
nit: reverseproxy/server.go:797 will drift with the next edit to that file. The function name requestScheme(r) is already right there, so the line number can go. Same in plugin_test.go:434.
There was a problem hiding this comment.
Fixed in 805529b — both of them, plugin.go and plugin_test.go:434. They
now read (reverseproxy/server.go) and (requestScheme, reverseproxy/server.go).
You called it exactly: requestScheme is at 797 as of this commit, and it only
has to survive one edit above it to start pointing at something unrelated —
which is worse than no reference, because a confidently wrong line number sends
the reader to the wrong function instead of making them grep. The symbol name
is stable and greppable, so the number was carrying no weight.
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com
Round-2 review follow-ups on #1537. Documentation and comments only; no behaviour change. The inbound prose named the wrong hop. The paragraph above it already explains that the credential does not travel on the client->Cortex hop, so the uncovered inbound risk is not the front listener's transport -- it is a reverse_proxy_backend of "http://", where the real key is injected and then crosses that hop in cleartext. mtls: on the front listener does not change that. Both the doc and the plugin.go comment carried the same slip; both now point at reverse_proxy_backend and say plainly that Cortex does not refuse an http:// backend. Also adds the shape the "Deployments this refuses" table was missing: under mtls: mode: strict the forward proxy dials every upstream through mtlsDialer (TLS-or-fail), so an http:// target is TLS on the wire, yet the gate reads the request scheme and still returns 403. Fail-closed and rare, but worth a row so the 403 is not a mystery. Drops two line-number references (reverseproxy/server.go:797) that would drift with the next edit to that file; the requestScheme function name is enough to find it. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Mariusz Sabath <mrsabath@gmail.com>
github-project-automation
Bot
moved this from New/ToDo
to Done
in Rossoctl Issue Prioritization
Brings in rossoctl#1537 and rossoctl#1539. Both sides added SessionEvent fields and forward-proxy imports: kept both. main's DialAddr took the field count to 30 and this branch's Proc, ClientProc and SessionHeader add three, so the merged struct has 33. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Resolves the conflict with main, which has since merged #1539 (the forward proxy dial refusal) and #1537 (the plaintext-credential refusal). Both conflicts were add/add in core/listener/egressguard and both resolved to this branch's side, which is the change the PR exists to make: - guard.go: the corrected zero-value Guard doc. The old "refuses nothing and dials nothing" text on main is what this branch set out to fix, so main's side is the stale one. - guard_test.go: the two zoned NAT64 rows and the comment recording what they measured. Additive; main's side simply predates them. The branch was cut before #1537, so merging it without this would have deleted that feature — the plaintext gate, its three test files, the staticinject plugin changes and its doc, about 600 lines. Those merged cleanly and are kept; the diff against main is now only the 24 lines this PR actually adds. Verified: full core suite with CGO_ENABLED=0, 0 failures, including the staticinject, extproc and reverseproxy packages #1537 brought in; go vet and gofmt clean on the touched packages. TestCheckAddr_ZonedV6StillRefused passes with the two NAT64 rows. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Mariusz Sabath <mrsabath@gmail.com>
main gained the egress guard (#1539) and the plaintext-credential refusal (#1537), which touch the same files as this branch. Resolved by keeping both sides: - forwardproxy/server.go: main's guard-checked dialTarget, dialed through this branch's dialUpstream seam instead of net.DialTimeout, so the WebSocket tests' dial hook still applies; both new imports. - pipeline/session.go: TunnelWebSocket and ErrorKindTurnInterrupted beside TunnelDestPrivate and ErrorKindDestinationPrivate. - cost/usage: turn_interrupted and destination_private both exempt from the error count. - config: this branch's responses_websockets in place of the decline_responses_websockets field main still had, plus main's EgressGuard field and its test. EgressGuard's comment no longer points at the removed field. - CLAUDE.md, docs/session-api.md: the WebSocket text beside main's egress-guard text, minus main's line describing the removed boolean. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Alan Cha <Alan.cha1@ibm.com>
Resolve conflicts from #1539 (egress guard + SanitizeControlRunes rename), #1537, and #1536 landing on main while this branch carried parallel edits to the same files: - core/pipeline/client.go: take main's evolved SanitizeControlRunes doc; drop this branch's now-superseded paragraph (body unchanged). - core/tlsbridge/upstream.go: keep this branch's WebSocket/requiresHTTP1 doc paragraph ahead of main's new variadic NewUpstreamClient signature and WithDialContext (egress guard dialer). - CLAUDE.md, docs/session-api.md: combine both sides' parallel doc additions — WebSocket forwarding next to the egress guard's 403 destination_private behavior. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Mariusz Sabath <mrsabath@gmail.com>
Brings in #1539 (egress guard, 403 destination_private) and #1537 (plaintext_credential), both now on main, and resolves the four conflicts they create with this branch: - core/listener/forwardproxy/server.go, cmd/cortex/main.go: additive import conflicts — main adds egressguard, this branch adds egressallow. Both are kept. - core/pipeline/client.go: both branches exported SanitizeControlRunes with its own doc comment. #1539 landed first, so main's comment stands and this branch's duplicate is dropped — the function body is identical. - CLAUDE.md: both document a 403 refusal in the same event-schema paragraph. Both are kept, plus a note on how they compose: the allow list judges the NAME before DNS, the guard judges the ADDRESS it resolved to at the dial, so an allow-list entry is not a permit for the guard. One conflict git could not see, because both sides ADDED the file: #1539's egressguard_test.go and this branch's egressallow_gate_test.go each define connectRaw, which is a redeclaration in package forwardproxy. The two bodies are identical, so this branch's copy is removed and the one on main is used; `bufio` and `net` go with it. `go vet` caught it — `go build` does not compile test files. Verified on the merge result: core go test 72 ok, the only failures being core/plugins and core/plugins/opa, which fail to build on this machine for lack of a cgo toolchain and do so on unmodified main too. cmd/cortex ok. gofmt, go vet and go mod tidy -diff clean on both touched modules. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Mariusz Sabath <mrsabath@gmail.com>
Closes #1481 (MOCA SBX epic #1490, P1).
Summary
A guest can send an absolute-form
GET http://…request through the forward proxy.static-injectnever readpctx.Scheme, so a credential resolved for that host would be injected onto a request that crosses the network in cleartext — exactly what MOCA's MI1 §8.5 item 3 forbids.This adds a plaintext gate to
static-inject: when the request's scheme ishttpand the resolved host has a credential, the plugin denies with 403plaintext_credentialbefore any header is written — nothing injected, nothing sent upstream. The denial also records adenyinvocation so the refusal is attributable in the session timeline.The issue's design question — check in every injector, or a single gate before any credential plugin runs — is answered in the injector, after resolution:
httpscheme only. An empty scheme (the listener couldn't determine one) is not guessed to be plaintext — matchingContext.Scheme's own contract that treating""as http would mask missing listener plumbing. TLS-bridged requests carryhttpsand inject as before.token-exchangeinjects short-lived tokens onto plaintext in-cluster calls by design, and its trust model differs fromstatic-inject's "the workload never holds the real secret".On the code string:
plaintext_credentialis the MOCA egress contract's identifier, used verbatim rather than the repo's usualstatic-inject.<kebab>form, so the conformance suite Cortex shares withmoca-egress(plaintextHTTPCredential403inremote-worker/egress/conformance) can match it without a translation table — the same reasoning that freezesx-authbridge-*wire names. The suite's wire shape (X-Moca-Egress-Errorheader, text/plain body) is moca-egress's own rendering; on the Cortex side the refusal surfaces as the standard JSON rejection body ({"error":"plaintext_credential",…}) plus the invocation record.Tests
core/plugins/staticinject/plugin_test.go— the contract case (403, code verbatim, header unmodified, deny invocation recorded); the negative space (plain HTTP to a host without a credential →401 static-inject.unresolved-key, not 403);httpspass-through; empty/tcpscheme pass-through.core/listener/forwardproxy/server_test.go— end-to-end: a proxied absolute-formhttp://request to a credentialed host gets 403 with theplaintext_credentialcode, the body carries no credential, and the upstream is never dialed.Docs
docs/static-inject-plugin.md— new "Plaintext refusal (403 plaintext_credential)" section: what fires, what deliberately doesn't, and why the gate lives in the injector.Assisted-By: Claude (Anthropic AI) noreply@anthropic.com