Repository navigation
Feat: Remember TLS bridge refusals per program, not per host - #1357
Conversation
A program that refuses the leaf three times in a row is passed through until a success or a restart, instead of failing again every time its window ends. The host-keyed set keeps never stopping. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
When the forward proxy can name the client's program, a refused leaf is remembered against that program, so another program on the same host is still bridged, and the program's next connection is passed through as program-refused. A client whose program cannot be named keeps the host memory exactly as before. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
…ejection The program memory is seeded by every failed forge, as the host memory is, so the reason's comment and its doc row no longer claim the program rejected the CA. The restart advice is now conditional on the earlier failure having been a rejection, and the sentence after the table says so. The headline test also pins that a refusing program's first connection is recorded as client-rejected-ca. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
The design stops a program after three refusals, "each after a wait in which the program could have been fixed". The program set counted every Fail instead, and both callers check Contains before forging, so the connections of one burst all pass that check before the first refusal is recorded. Three of them stopped a program for the rest of the run in under a second. On the program set, a Fail that lands while the entry's window is still open is now held like a transient failure: it neither raises the count nor shortens the window. The host set never stops, so it keeps escalating on a burst exactly as before. The program-set tests that stopped an entry with back-to-back Fails now space each counted refusal past the previous window, and TestProgramSkipSet_ABurstCountsOnce pins the burst rule. The transient test now spaces its failures too: back to back, the burst rule would hide a FailTransient that escalated. Also corrects comments the change makes stale: the eviction sweep keeps stopped entries, an expired entry is no longer where every rejection after the first lands, and a stopped entry is passed through until Cortex restarts, since nothing bridges it and so no success can clear it. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
… process The program key is (Exe, Agent), and an agent's Agent half stays empty until that process names a session through a decrypted request. The most common refusal is a stale client, started before the current CA existed: a first install, a recreated ~/.cortex, a CA renewal. A stale Claude Code therefore has the key a freshly started Claude Code has before it names a session. Once the stale one stopped that key, every new instance was passed through as program-refused, never named a session, and was never read until Cortex restarted. The documented advice, restart clients started before ca_not_before, stopped working. A process that started before the bridge CA's NotBefore cannot have been told about the CA, so its refusal is evidence about that process, not the program. Such a refusal is now recorded under Program.ProcessKey, the program key plus "<pid>@<start>" after a second NUL, so it can never equal a program key, and the two kinds of key are told apart by how many fields a split on NUL gives. A connection is passed through when either key is held, a success clears both, and an unknown start or NotBefore records against the program as before. caNotBefore now renders from the same parse that stale-process check uses, so the cutoff the log advises against is the one the code applies. The rejection line also names the agent when there is one, since python3 under an agent and python3 alone are remembered apart but logged the same. The fake processes in the tests start in 1970, before any CA, so the tests about an ordinary program now use processes started after it. TestBridgeProgram_AStaleProcessDoesNotStopItsProgram pins the fix end to end. Docs and the program-refused comment say a stop lasts until Cortex restarts, that a process older than the CA is remembered on its own, and that restarting that process is enough. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue is established; the change is mergeable after normal checks. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: TLS bridge refusal memory now tracks identified clients per program instead of per host. |
| Linked Issues check | |
[#912] The forward proxy uses per-program and per-process keys for named clients. It passes a program through after three counted refusals and preserves host-based behavior for unnamed clients. The te… |
| Out of Scope Changes check | |
The changes stay within TLS bridge refusal handling. The production changes wire program memory, derive program identity, classify named and unnamed CONNECT clients, record tunnel reasons, update reje… |
| Docstring Coverage | |
Docstring coverage is 83.87% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 31 functions across 11 files. (1 skipped: 1… |
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
esnible
left a comment
esnible
left a comment
There was a problem hiding this comment.
The change does what it says. Unnamed clients and Engines built without Programs keep exactly the old path, and the burst, transient, stop and eviction rules in SkipSet.fail hold up when traced. The tests cover each rule end to end over real TLS. Locally, go test -race passes on tlsbridge, forwardproxy and pipeline; the timing-based tests pass with -count=5 and with -count=20 -cpu 1; gofmt and vet are clean. Nothing blocks merging. The two suggestions are a design doc the code cites that isn't in the repo, and how expensive it is to recover from a stop.
Author: huang195 (MEMBER — maintainer)
Areas reviewed: Go (core/tlsbridge, core/listener/forwardproxy, core/pipeline, cmd/cortex), Docs
Agent/IDE config (.claude/.vscode): none
Commits: 5 commits, all signed off: yes
CI status: passing (Spellcheck skipped)
| } | ||
|
|
||
| // bridgeVerdict applies the TLS bridge's rules to a CONNECT, in the order the design | ||
| // gives them (docs/superpowers/specs/2026-10-09-tls-bridge-per-program-design.md): the |
There was a problem hiding this comment.
suggestion: This cites docs/superpowers/specs/2026-10-09-tls-bridge-per-program-design.md, but that file isn't in this PR, on main, or in #1358 or #1359. bridge_program_test.go:23 cites the same path. Please add the spec to this PR or drop the path. As it stands, "in the order the design gives them" sends readers to a document they can't find.
| // say it will not change during this run; retrying forever is what made one refusing | ||
| // client fail again every ten minutes (#912). A rejection that lands while a window is | ||
| // still open does not count toward it (see fail). | ||
| const programStopAfter = 3 |
There was a problem hiding this comment.
suggestion: ProcessKey covers a process that started before the CA. Any other refusing instance of an agent still stops <exe>\x00: the #1033 sandbox, a different CLAUDE_CONFIG_DIR, or one started inside the CA's one-minute backdate (see the comment on bridgeMemory).
For an agent's own executable, that stop keeps itself in place. Every new instance shares <exe>\x00 until it names a session. A passed-through instance is never decrypted, so it never names one. So no new instance is read until Cortex restarts, and that restart cuts every attached session. The fastest path to a stop is about 90s (30s + 60s windows).
This isn't blocking, since the PR lists the sandbox case under costs. But a way to clear one key without a restart (#1359's list seems the natural home) would make the fix much cheaper than it is now.
| if tl == nil || tl.program == nil || s.TLSBridge.Programs == nil { | ||
| return s.TLSBridge.Skip, host | ||
| } | ||
| if nb := s.caNotBeforeTime(); tl.program.Start != 0 && !nb.IsZero() && time.Unix(0, tl.program.Start).Before(nb) { |
There was a problem hiding this comment.
nit: ca.go sets the CA's NotBefore a minute before it was minted (time.Now().Add(-time.Minute)). So a process started up to 60s before the CA existed passes this check as fresh, and its refusal lands on Key, which is exactly the case ProcessKey exists for. The window is narrow, but erring toward stale is the cheap direction: at worst, that one process is remembered on its own. A minute of slack here, or a sentence saying the cutoff is approximate, would help. The ca_not_before advice in the rejection line has the same slack.
| } | ||
| } | ||
|
|
||
| // Review focus 3. The same interpreter run by an agent and run on its own can disagree |
There was a problem hiding this comment.
nit: "Review focus 3" (and 1 and 2 in this file, plus 5 at tlsbridge/decision_test.go:587) points to a review checklist that isn't in the repo, so the numbers will mean nothing to a later reader. The sentence after each label already says what the test is for, so I'd drop the labels.
| | `passthrough-nontls` | The bytes were not a TLS handshake, so there was nothing to terminate. | no | | ||
| | `skip-cached` | An earlier handshake for this host failed, so it is not intercepted for **anyone** for a short window. Any failed handshake seeds this, not only a CA rejection — the seeding failure logged its own reason. The window starts at 30s and lengthens only if **rejections** keep coming — a hang-up or a cipher mismatch seeds it but never escalates it; the first client that *does* trust the CA clears it immediately. | find the earlier failure in `proxy.log` and fix that client | | ||
| | `skip-cached` | An earlier handshake for this host failed, so it is not intercepted for a short window. This applies only to a connection whose program Cortex cannot name — a failed process lookup, or `session.process_attribution: off`; when it can name the program, that program's own record decides instead (`program-refused`). Any failed handshake seeds this, not only a CA rejection — the seeding failure logged its own reason. The window starts at 30s and lengthens only if **rejections** keep coming — a hang-up or a cipher mismatch seeds it but never escalates it; the first client that *does* trust the CA clears it immediately. | find the earlier failure in `proxy.log` and fix that client | | ||
| | `program-refused` | An earlier handshake by the program that opened this connection failed, so Cortex did not try to read this one. Only that program is affected: another program talking to the same host is still read. Usually the earlier failure was a CA rejection, but any failed handshake seeds this — the `proxy.log` line for that failure names the program (`program=`, and `agent=` when it runs under one) and logged its own reason. Cortex tries again after a window that starts at 30s and lengthens only if **rejections** keep coming — a hang-up or a cipher mismatch seeds it but never counts, and nor does a rejection arriving while the window is still open. After three rejections in a row, each after a window, Cortex stops trying for that program **until Cortex restarts**. A process that started before Cortex's CA (`ca_not_before`) is remembered on its own instead, not as its program: it cannot have loaded that CA, so restarting that process is enough, and a newly started copy of the program is read meanwhile. | find the earlier failure in `proxy.log`; if it was `client-rejected-ca`, restart the client if it started before `ca_not_before`, otherwise point it at the CA, then restart Cortex | |
There was a problem hiding this comment.
nit: The last cell reads as if "then restart Cortex" applies to both branches. A restart is only needed once the program is stopped (three rejections). Before that, pointing it at the CA and waiting out the window is enough. A Cortex restart cuts every attached session, so it's worth saying when it's needed, e.g. "…otherwise point it at the CA; if it is still program-refused a couple of minutes later, restart Cortex."
3 participants
First of three PRs for #1348 and #912. Design: per-program TLS bridge decisions.
The problem
Cortex reads HTTPS by showing a program a certificate signed by its own CA. That only works if the program trusts the CA, and some never will. Two things went wrong:
Whether a program trusts Cortex depends on the program, not the host. The forward proxy already knows which program opened each connection (
core/peerproc, #1229); it just wasn't used for this.What changes
When the forward proxy can name the client's program, the bridge decides and remembers per program:
program-refused. Another program talking to the same host is still read.tlsbridge.Program). Sopython3run by Claude Code, which inherits the CA variables, is tracked apart frompython3run by cron, which doesn't.Program.ProcessKey). Without this, a stale Claude Code — the usual refusal after a first install or a CA renewal — would stop the key every freshly started Claude Code shares until it names a session, and no new instance would be read. The existing advice to restart the stale client keeps working.program=, andagent=when there is one).What does not change
session.process_attribution: off, a failed lookup, and the transparent listener.Enginebuilt withoutProgramskeeps today's behaviour too, as any outside consumer ofcoremay build one.DefaultPassthroughHosts) is unchanged.Departures from the design, on purpose
skip-cachedrow's promise that the first client which trusts the CA clears it, and only helps clients whose program can't be named.Costs accepted
$HOME, bug: a per-sandbox $HOME gives each sandbox its own CA, and the client only hears "Self-signed certificate detected" #1033) can still stop its program's key. PR 3's list will show it, and a Cortex restart clears it.Tests
core/tlsbridge:KeyandProcessKeynever collide.core/listener/forwardproxy, end to end over real TLS with a fake process table:client-rejected-ca, its next one is passed through asprogram-refused, and a trusting program to the same host is still bridged;go test -race ./...incorepasses, andcmd/cortexbuilds and vets.Minor items left for later
FailTransient, or a set full of nothing but stopped entries. Both traced correct.TestProgramKey's second assertion only catches a missing separator.Classify, so a CONNECT to a skipped host now gets one cached process lookup it didn't before.peerproc.Proc.Startfrom a third-party resolver reads as a process older than the CA. The resolvers in this repo always set it.program-refusedrow says a hang-up "never counts". But an entry first created by a hang-up starts its count at one, so the stop can come one rejection sooner. PR 2 corrects the wording.Coming next
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com
Summary by CodeRabbit