Compile protected Codex policies and reserve managed grant inputs

[jhgaylor]

Fountain's legacy ChatGPT compiler puts the managed bearer in the same map used by ordinary custom templates. This adds a separate protected compiler for ADR 52 and reserves the managed key/placeholder on binding, environment-secret and vault-secret writes. It pins the published broker 0.14.0 release.

The new compiler accepts a typed Grant separately, rejects copied bearer values and reserved references in ordinary inputs (including older persisted bindings and network patterns), and rejects overlapping ordinary injection rules. It produces a fixed HTTP-only policy for POST https://chatgpt.com:443/backend-api/codex/responses, with a pinned account header and explicit header allowlist. Ordinary secret templates and limited-network deny behavior are preserved. No managed bearer or authorization reference is emitted in the compiled policy.

This is stacked on #2015 at 1c43d049. The compiler is intentionally not connected to production session issuance: durable grant/session fencing, per-request authorization, source-specific auth homes, existing-data preflight and fleet-wide legacy socket draining remain prerequisites. Existing platform sessions still use the legacy path; user linking remains disabled.

Validation:

• Final commit 6f6fdb8a: CI required and coverage gates passed, covering 5,727 tests across all six core/EE partitions and both extension apps. Static analysis, release boot/API contracts, SDK/CLI checks, both Swift jobs, core distribution and Compose checks also passed. CI run.
• Local mix precommit with the four affected compiler, reserved-write, binding and broker test files passed: 79 tests, zero failures, plus compile, formatting, Credo, Dialyzer, Sobelow and production release assembly. The complete-suite result is from CI.
• A reproducible probe ran codex-acp 1.10.0 with Codex CLI 0.153.4 through two synthetic prompts in the same ACP session. Both completed and sent zstd JSON POSTs with Content-Length and the expected synthetic bearer/account pair. Checked-in metadata is replayed through broker policy/header preparation in ExUnit; no real provider tokens or request bodies are recorded.
• The fixture proves local client request shape and SSE consumption. Real provider acceptance, production HTTPS/proxy behavior, optional Codex features and the deployed CLI version remain rollout gates. The adapter depends on @openai/codex ^0.153.3, so pinning the adapter alone does not fix the CLI patch version.

[BinaryBourbon]

Scope: I reviewed the broker/policy half only — protected_compiler.ex, grant.ex, the broker 0.14.0 pin, protected_compiler_test.exs, the codex_protected fixture and scripts/probe-codex-protected.py. The reserved-input half (reserved.ex, environments/secret.ex, secret_bindings/binding.ex, vaults/vault_secret.ex) is covered by a separate review.

Approving. I found nothing that makes the compiled policy permissive or lets the managed bearer reach a destination other than the pinned one. Notes below are non-blocking.

What I verified by running it

The bearer is not in the output, and there is no authorization reference. Compiled with a marker token and dumped the result:

%{protected: %ProtectedRule{name: "codex-chatgpt", host: "chatgpt.com", port: 443,
    paths: ["/backend-api/codex/responses"], methods: ["POST"],
    identity: "acct-1", identity_header: "chatgpt-account-id", allowed_headers: [...]},
  rules: [], http_only: true, unmatched_host_policy: :passthrough}
token in term_to_binary? false

No :authorization key, and ProtectedRule.valid_session?(struct(Session, compiled)) is false — so a session built straight from this output is rejected at proxy.ex:383 (lookup returns :error). The output cannot serve traffic on its own, as the moduledoc says.

It fails closed on every malformed input I could construct. identity of nil, "", 123, :atom, "a b", a NUL byte, or "acct\r\nX: y" → :invalid_managed_identity (the CRLF case is the one the test pins). A grant that is nil, %{}, has access_token: ""/nil/an atom, or whose source lacks account_id → :invalid_managed_grant. network of nil, :deny, {:unknown, []}, {:limited, "notalist"} and bindings of %{"K" => nil}/[nil]/"notalist" → :invalid_broker_configuration (via the whole-body rescue). An unknown extra field in source is ignored and compiles, which is the right call. I could not find an input shape that produces a permissive policy.

Ordinary rules cannot widen the destination. Bindings for chatgpt.com, *.com, chatgpt.com:443, chatgpt.com/backend-api/*, CHATGPT.COM (uppercase) and a enabled: false binding on the pinned host all fail with :managed_destination_conflict. A non-overlapping rule (chatgpt.com/v1/) compiles clean — but I confirmed that is safe rather than a hole: ProtectedRule.select/2 sends every request whose host is chatgpt.com down the protected branch or refuses it, so the ordinary rule can never fire. select on /v1/x returns {:error, :protected_destination}. Scheme, port, method, path traversal, %72esponses, // and -export suffixes are all refused.

The header allowlist holds under casing and duplicates. Sending Authorization, AUTHORIZATION, Chatgpt-Account-Id, CHATGPT-ACCOUNT-ID, Cookie and X-Forwarded-Host through prepare + inject yields exactly one authorization (the managed bearer) and one chatgpt-account-id (the pinned identity); the client's copies are gone. Identity mismatch, a bearer with a space, and a nil bearer all return :authorization_unavailable.

CRLF cannot manufacture a header. ProtectedRule.prepare does not validate header values — {"version", "1\r\nAuthorization: Bearer leaked"} survives it. It is caught one layer up: proxy.ex:1043 runs HTTPOnly.check_request on the raw request before select/prepare/authorize_protected, and that returns {:error, :unsafe_request} for the same header list (verified directly). So the refusal happens before the credential is ever resolved.

ADR 0052's broker requirements. Upgrade rejection before injection, fail-closed on an unexpected upstream 101, and HTTP-only are implemented in broker 0.13/0.14 (HTTPOnly.check_request, accept_head(_, %{status: 101}, ...) -> {:error, :upstream_upgrade}) and this PR opts in with http_only: true. Per-request authorization, durable owner/generation fencing and fleet-wide legacy socket draining are deferred, and both the moduledoc and the fixture README say so explicitly rather than claiming them. No overclaim found.

The bump. 0.11.0 → 0.14.0, fetched and diffed against the vendored 0.14.0: rule.ex, injector.ex, http.ex, ca.ex, certs.ex are byte-identical; proxy.ex (239 changed lines), session.ex, store.ex, response.ex changed. apps/fountain is the only consumer in the umbrella. Existing sessions keep authorization: nil / http_only: false / protected: nil, so every new gate is inert on the current path. Store.authorize_protected/3 rescues a missing optional callback into :authorization_unavailable, so the un-wired state fails closed too. Local test/fountain/broker + test/fountain/chatgpt_accounts pass (21 tests), and CI is green on all six partitions.

The tests are load-bearing. Seven revert-checks, each restored afterwards; every one fails:

Mechanism removed	Failing test
contains_bearer? from conflicting_inputs?	reserved keys / network smuggling (2)
Reserved.conflict?	reserved keys / network smuggling (2)
ProtectedRule.prepare conflict check	exact/wildcard/path conflicts
valid_session? gate	malformed inputs
ordinary_secrets? gate	network patterns and nested invalid inputs
x-codex-window-id from @headers	the captured two-turn ACP contract
http_only: true in the output	fixed policy contains identity

The last two matter most: the fixture is genuinely exercised, not decorative, and http_only is asserted.

Probe and fixture hygiene. capture.json is 1.7 KB of header names, media types and booleans — no bodies, no tokens, no tenant data. The probe uses synthetic-managed-token / account-fixture / fixture@example.invalid against a loopback origin under an isolated CODEX_HOME, and hard-fails on version drift, an incomplete turn, a route/method change or a missing Content-Length. Nothing real is checked in.

Non-blocking notes

1. Four allowlisted headers the audited client never sent. The fixture observes 12 client headers; @headers has 16. accept-encoding, version, x-openai-subagent and x-codex-turn-state appear in neither captured request. They come from the source audit the README cites, which is defensible — but x-codex-turn-state is interesting: the probe's origin does send it as a response header (probe-codex-protected.py, do_POST), evidently expecting the client to echo it, and the capture shows it did not. Either trim to what was observed, or say in the moduledoc that the list is "what this CLI version can emit" rather than "what it does". These are tenant-reachable values forwarded to chatgpt.com, so the list is worth keeping tight.

2. prepare neither de-duplicates nor normalizes surviving headers. Two session-id values and both version and VERSION pass through verbatim. Reserved names are stripped case-insensitively (verified), so there is no auth smuggling here — the residual is duplicate-header ambiguity at the origin. Library-side; mentioning it for the record.

3. safe_identity? has no length bound. A 5,000-character account_id compiles and becomes a header value. It is provider-derived rather than tenant input, so low risk, but a bound would cost nothing.

4. The rescue wraps the case on ProtectedRule.prepare. Only {:ok, _} and {:error, :protected_conflict} are matched; anything else becomes :invalid_broker_configuration through the rescue rather than a CaseClauseError. That is fail-closed and exhaustive for headers: [] today, but a future library error would be silently reclassified. Worth a comment at minimum.

5. Rollout note, not a defect. Once a protected session is issued, a tenant loses all other routes to chatgpt.com — a legitimate binding to another path on that host compiles fine and then gets :protected_destination at runtime. Likewise unmatched_host_policy: :deny cannot block the pinned destination, since select runs ahead of rule matching. Both follow from the design; they belong in the existing-data preflight the ADR already lists.

6. Reading-based, colleague's half: Reserved.conflict?/1 catches the placeholder only incidentally — String.downcase("__codex_chatgpt_access_token__") happens to contain String.downcase(@key). It works, but it reads as a coincidence rather than an intent; an explicit placeholder check would survive a rename of either constant. Flagging for their review, not asking for a change here.

Everything above is empirical except notes 5 and 6 (reading) and the CI/upstream-suite statements (CI output).

[BinaryBourbon]

Scope: I reviewed the reserved-input half only — chatgpt_accounts/reserved.ex, environments/secret.ex, secret_bindings/binding.ex, vaults/vault_secret.ex and reserved_test.exs. The broker/policy half (protected_compiler.ex, grant.ex, the 0.14.0 pin, the fixture) is the separate review above, and I agree with its conclusion where the two touch.

Approving. I could not find a fourth tenant-reachable write path or a normalisation bypass, and every guard in this half is load-bearing under revert. Two notes below; neither is introduced by this PR, and the first is the one I would not let slip.

What I verified by running it

Every writer of the three tables goes through a guarded changeset. I grepped lib/, ee/lib/, apps/fountain_buzz/lib and apps/fountain_support/lib for writers of secrets, vault_secrets and secret_bindings. There are exactly five entry points, and all five funnel into the changesets this PR guards:

Entry point	Reaches
SecretController / EnvironmentsLive.Form	Environments.upsert_secret/4 → Secret.changeset/3
VaultSecretController / VaultsLive.Form	Vaults.upsert_secret/4 → VaultSecret.changeset/3
SecretBindingController / SecretBindingsLive.Index	SecretBindings.create_binding/3, update_binding/3 → Binding.changeset/2
Fountain.Manifest (the bulk chant apply import, manifest.ex:199,225)	the same two upsert_secret/4
FountainBuzz (fountain_buzz.ex:262)	Vaults.upsert_secret/4

No Repo.insert_all, Repo.update_all or direct Repo.insert(%Secret{}/%VaultSecret{}/%Binding{}) exists against these tables outside the sweeper's expiry_notified_at stamp (secret_expiry_sweeper.ex:92, which touches neither key nor value). VaultSecret.metadata_changeset/2 casts only :expires_at, so it cannot carry a name. The bulk import path was the one I most expected to bypass the changeset; it does not.

The guards sit in the right place in each pipeline. In Secret/VaultSecret the check runs before put_ciphertext(dek) (secret.ex:29, vault_secret.ex:31), so :value is still plaintext when it is inspected. In Binding it runs after update_change(:key, &String.trim/1) and after validate_auth_fields/1 (binding.ex:78), so it inspects the effective, post-clear/post-default values that will actually be persisted rather than the raw attrs. That ordering also means an api_key binding whose headers are cleared to %{} is not rejected for a value that never lands — correct, not a hole.

Normalisation. I ran Reserved.conflict?/1 over a table of candidates:

"CODEX_CHATGPT_ACCESS_TOKEN"              => true
"codex_chatgpt_access_token"              => true
"__codex_chatgpt_access_token__"          => true   # the placeholder
"__CODEX_chatgpt_ACCESS_token__"          => true
" CODEX_CHATGPT_ACCESS_TOKEN "            => true
"$${CODEX_CHATGPT_ACCESS_TOKEN}"          => true
"CODEX_CHATGPT_ACCESS​TOKEN"         => false
"CОDEX_CHATGPT_ACCESS_TОKEN" (Cyrillic О) => false

The substring-on-downcase check subsumes the placeholder (the placeholder contains the key), so guarding one name covers both; whitespace, quoting, embedding and the $$ escape are all caught because containment does not care about the surroundings. The two confusable cases are not bypasses: split_inference/2 decides with Map.has_key?(bindings, "CODEX_CHATGPT_ACCESS_TOKEN") — a byte-exact match — and @key_re ~r/^[A-Z][A-Z0-9_]*$/ on binding.ex:69 rejects a non-ASCII key before the reservation is reached. I also checked assembling the placeholder from two concatenated secret values: Managoat.Substitution's @ref only expands ${VAR} from vars, vars never contains the managed credential (merge_secrets/3 is env + vault secrets only), and a value assembled inside a sandbox is still only a placeholder — the broker resolves it on chatgpt.com and nowhere else.

The tests are load-bearing. Eight revert-checks, each restored afterwards. Every one fails reserved_test.exs:

Mechanism removed	Result
the guard line in environments/secret.ex	1 failure
the guard line in vaults/vault_secret.ex	1 failure
:value narrowed out of the Secret field list	1 failure
:headers dropped from the binding field list	1 failure
:prefix dropped from the binding field list	1 failure
:key dropped from the binding field list	2 failures
conflict?/1 on a binary always false	3 failures
conflict?/1 loses its String.downcase	2 failures

All three write paths are covered, per-field rather than per-call, and the second test exercises the pre-existing-row shape (Repo.insert! then update_binding) including the repair-and-delete escape hatch. This is a materially stronger test file than the one earlier in this stack that passed with its mechanism removed.

Error behaviour and audit. A rejected write returns {:error, %Ecto.Changeset{}} with "is reserved for managed ChatGPT credentials" and records nothing: audited_secret(other, ...) (vaults.ex:253, environments.ex:265) and audited(other, ...) (secret_bindings.ex:118) pass non-:ok tuples straight through, which is CLAUDE.md's "only record what happened". The successful-write metadata is key/host/auth_type only — names, never values — and Audit.record/1 is called outside any transaction. The error message names no value and echoes nothing back.

Vault precedence. Vault-wins-on-collision does not open a seam here: both Secret.changeset/3 and VaultSecret.changeset/3 carry the identical guard over the identical field pair, so there is no asymmetry for a vault to exploit at SpriteEnv.merge_secrets/3.

Non-blocking findings

1. Existing binding rows are still live, and nothing in the read path filters them

This is the deferred "existing-data preflight" the PR body names, so the code does not over-claim — but it is worth stating precisely what stays open, because the boundary this PR is named for is only half closed for tenants who already have a row.

Reserved is referenced in exactly four places in lib/: the three changesets and protected_compiler.ex:100. Nothing filters on read. SecretBindings.enabled_by_key/1 (secret_bindings.ex:31) is list_bindings |> filter(& &1.enabled) |> group_by(& &1.key) with no exclusion, and there is no migration against secret_bindings. I inserted a row directly and ran the real split:

enabled_by_key             => ["CODEX_CHATGPT_ACCESS_TOKEN"]
Broker.split_inference(%{codex_chatgpt_access_token: "REAL-TOKEN"}, bindings)
  creds                    => %{codex_chatgpt_access_token: "__codex_chatgpt_access_token__"}
  brokered                 => %{"CODEX_CHATGPT_ACCESS_TOKEN" => "REAL-TOKEN"}
  implicit["CODEX_..."]    => nil            # the chatgpt.com binding was suppressed
  tenant binding hosts     => ["attacker.example"]

The real platform bearer is handed to the broker bound to a tenant-chosen host. That is live on main today — the @inference entry and its implicit binding landed in #1755 (ADR 0047), which is deployed — so this PR is a strict improvement and introduces nothing. Once ADR 0052 is connected, conflicting_inputs?/2 fails such a row closed at compile time, which is the right end state.

What I would ask for before the preflight lands: a one-line defensive reject in enabled_by_key/1, e.g. |> Enum.reject(&Reserved.conflict?(&1.key)). It closes the legacy path today at near-zero cost, is independent of the preflight (which is about telling operators which tenants are affected, a slower and more careful job), and leaves the row in place for the tenant to repair through the escape hatch this PR's second test already proves works. Your call whether that belongs here or in the next slice, but it should not wait for the preflight.

2. Environment.env_vars is a fourth tenant-controlled config field that can name the reserved key

reserved.ex's moduledoc says "Configuration may neither name the managed input nor embed its placeholder." env_vars is configuration, is cast freely (environment.ex:60) and is unguarded:

Environments.create_environment(%{"env_vars" => %{"CODEX_CHATGPT_ACCESS_TOKEN" => "__codex_chatgpt_access_token__"}})
  => {:ok, env}    # accepted

I chased whether it matters and it is low severity, not an escalation. env_vars never reaches the brokered map — merge_secrets/3 is decrypted_env over env + vault secrets only, and Broker.split/2 runs on that map — so it cannot redirect the credential. The effect is confined to the sprite env list, where env.env_vars is appended after CodexChatGPT.env/2 (sprite_env.ex:117,121):

entries for the managed key, in order =>
  [{"CODEX_CHATGPT_ACCESS_TOKEN", "__codex_chatgpt_access_token__"},
   {"CODEX_CHATGPT_ACCESS_TOKEN", "TENANT-OVERRIDE"}]
List.keyfind (what prepare_sandbox reads) => the placeholder
last-wins (what the sandbox process sees)  => "TENANT-OVERRIDE"

So auth.json is still written with the placeholder and the grant path keeps working; only the process env var is shadowed, in the tenant's own sandbox. Worth adding Reserved.validate_changeset/2 over :env_vars in Environment.changeset/2 for consistency with the contract the moduledoc states, but I would not hold the PR for it.

3. Nit

conflict?/1 falls through to false for atoms, so :codex_chatgpt_access_token as a map key is not caught. Not reachable from tenant input (JSON decodes to string keys, and the typed %Grant{} is caught by its own clause), so this is hardening only — one extra clause if you want the struct-walk to be total.

Not my half

protected_compiler.ex:100's Reserved.conflict?(inputs) is the fail-closed backstop for finding 1 once ADR 0052 is connected; the review above covers that side and I found nothing to add to it.

CI is green on the final commit: all six partitions, coverage, static analysis, release boot, SDK/CLI and both Swift jobs. reserved_test.exs passes locally (3 tests) on a clean tree.

[BinaryBourbon]

Follow-up to my reserved-input review above. A cross-half lead was raised — that Reserved.conflict?/1 catches the placeholder only incidentally, because the placeholder happens to contain the key string — and I tested it. The lead is refuted as stated, but it is pointing at a real hole one constant over. Evidence for both.

Refuted: the placeholder containment is structural, not coincidental

Broker.placeholder/1 (broker.ex:243) derives the placeholder rather than storing it:

Map.get(@inference_prefix, key, "") <> "__" <> String.downcase(key) <> "__"

Any placeholder it can produce necessarily contains String.downcase(key), so conflict?/1 checking the key subsumes the placeholder by construction. Confirmed at HEAD:

Reserved.placeholder()                     = "__codex_chatgpt_access_token__"
Broker.placeholder(Reserved.key())         = "__codex_chatgpt_access_token__"
equal?                                     = true
Broker placeholder contains downcase(key)? = true

And the tests do pin it. Rewriting @placeholder to "__managed_bearer__" — a value no longer containing the key — fails immediately:

12 tests, 4 failures

So a placeholder rename is caught loudly, not silently. The second bullet (a reference that avoids the literal key and still resolves at merge time) I had already covered: $$, casing, partials and concatenation either still contain the key or never resolve to the credential, since vars at merge time is env + vault secrets only.

Confirmed, one constant over: Reserved.@key vs Broker.@inference can drift, and nothing catches it

The credential name is written out as four independent string literals in lib/:

chatgpt_accounts/reserved.ex:11	@key "CODEX_CHATGPT_ACCESS_TOKEN"
broker.ex:277	the @inference map key — the one that actually decides
conversations/codex_chatgpt.ex:32	@env_key
conversations/codex_transport.ex:63	@chatgpt_key

Nothing couples them, and every test that exercises the reservation feeds it Reserved.key() as both the input and the expectation (reserved_test.exs:22,45,69, protected_compiler_test.exs:68,103). A self-referential assertion cannot notice that Reserved.@key no longer names the credential the broker brokers.

I renamed the credential in the three non-reserved.ex copies to CODEX_CHATGPT_SESSION_TOKEN and left reserved.ex alone — the exact shape of a rename that misses one file:

--- do the reservation suites stay GREEN under the rename? ---
12 tests, 0 failures

--- and is the boundary open? ---
REAL key in Broker.@inference = "CODEX_CHATGPT_SESSION_TOKEN"
Reserved.key()                = "CODEX_CHATGPT_ACCESS_TOKEN"
Reserved.conflict?(real_key)  = false
BINDING ON THE REAL KEY WAS ACCEPTED          <-- boundary OPEN
brokered             => %{"CODEX_CHATGPT_SESSION_TOKEN" => "REAL-TOKEN"}
implicit[real_key]   => nil
tenant binding hosts => ["attacker.example"]

Both reservation suites stay fully green while the reservation guards a name nobody uses and a tenant binds the real platform bearer to a host of their choosing. That is the whole authorization boundary opening with zero test failures.

Not blocking, and what I would add

The constants agree at 6f6fdb8a — I verified that first, and the guard works today. This is a latent hazard, not a live hole, so it does not change my approval. But it is cheap to close permanently, and the cost of not closing it is that the failure mode is silent:

# reserved_test.exs
test "the reserved name is the credential the broker actually brokers" do
  assert Reserved.key() in Map.keys(Fountain.Broker.inference_keys())
  assert Reserved.placeholder() == Fountain.Broker.placeholder(Reserved.key())
end

Two lines, and it converts the rename above from silently-green into a red build. Better still, def placeholder, do: Fountain.Broker.placeholder(@key) removes the duplicated placeholder literal outright and leaves one assertion to carry the key. Either is fine; the first is the smaller diff and needs no thought about compile-time coupling between the two modules.

Worktree clean at 6f6fdb8a68ca6f464b5b2ddcf5ed6240068d67dd; every edit above was reverted and verified (git status --porcelain empty).

[BinaryBourbon]

Correcting note 6 of my broker/policy review above: the "reads as a coincidence" framing was wrong. Broker.placeholder/1 (apps/fountain/lib/fountain/broker.ex:243) derives the placeholder as prefix <> "__" <> String.downcase(key) <> "__", so any placeholder it can produce necessarily contains the downcased key — Reserved.conflict?/1 checking the key subsumes it by construction, and a rename is not silent. My approval stands.

The duplicate that is actually fragile is one constant over — reserved.ex:11 against broker.ex:277 — and it is established, with evidence, in #2017 (comment). That is the version to read; it is their finding, not mine.