feat(console): serve the egress allowlist iron-proxy already knows how to enforce#1067
Open
aadityakbh wants to merge 2 commits into
Open
feat(console): serve the egress allowlist iron-proxy already knows how to enforce#1067aadityakbh wants to merge 2 commits into
aadityakbh wants to merge 2 commits into
Conversation
…w to enforce
The proxy_sync controller has said this for a while:
The top-level `rules` ... intentionally omitted: centaur-console has no
models for them yet. Each secret still carries its own per-secret `rules`.
The consequence is larger than the comment suggests. iron-proxy only builds an
`allowlist` transform when the control plane sends `rules` (config/sync.go:
`if isNonNullJSON(rules)`). No rules served, no allowlist transform, nothing
rejects -- so a managed-mode sandbox reaches any host on the internet. The
per-secret rules do not close this: they scope where a credential may be
INJECTED, never where the workload may CONNECT.
This gives console the model it says it lacks, and populates it from two sources
because neither suffices alone:
derived -- every host a granted credential is already scoped to. This is what
makes the allowlist self-maintaining: grant a tool its secret and its host
is allowed. There is no second list to forget, and no new tool that 403s in
production with CI green.
base -- the hosts that carry no credential and so appear in no rule: the
model provider, package registries, the git host. Derivation is blind to
them, and a sandbox that cannot reach them cannot run.
Derived from GRANTED credentials, not the served subset. served_credentials
drops any secret whose source is currently undeliverable; deriving from it would
let a vault blip silently rewrite the egress boundary and 403 a host that is
still authorized. Losing a credential should cost the request its credential,
never its destination.
Three properties worth stating, because each is a way this could go wrong:
- Off by default. An existing deployment upgrades with its egress behavior and
its config_hash byte-for-byte unchanged.
- Omitted is not empty. `rules: []` is not "no allowlist" -- it is an allowlist
that matches nothing and 403s every request, including the one to the model
provider. The key is absent when off, never null and never empty.
- Turning it on with an empty base list is refused by a validation, because
that is the same blackout arriving by a different road.
`warn` mode serves the rules but tells the proxy to log what it would have
blocked rather than block it -- the audit pass before enforcing. It needs the
companion iron-proxy change that carries `allowlist_warn` through the sync
payload; until that lands, warn serves rules that the proxy enforces, so leave
it off.
Tests: 11 new, and the full console suite (1211 runs) is green.
…control plane Caught while standing the release up, which is the only place it could have been caught. Since paradigmxyz#1002, sandbox -> api traffic rides iron-proxy. So the API host has to be in the allowlist, or the sandbox cannot reach api-rs at all and EVERY TURN DIES. The allowlist would take the whole bot down rather than bound it -- the failure mode a default-deny egress control most needs to not have. The API-server JWT is a GENERATED proxy secret, not a granted one: it is minted per principal, so there is no Grant row to derive from. egress_rules derived only from granted credentials, so it missed the one credential whose host is load-bearing for the sandbox being able to function at all. It already carries `rules` in the right shape (api_server_hosts). Union them in. The test asserts the control-plane host survives into the allowlist. Verified it fails without the derivation and passes with it. Full console suite: 1212 runs green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
proxy_sync_controller.rbhas said this for a while:The consequence is larger than the comment suggests. iron-proxy builds its
allowlisttransform only when the control plane sendsrules:No rules served → no allowlist transform → nothing rejects. A managed-mode sandbox can reach any host on the internet. The per-secret rules don't close this: they scope where a credential may be injected, never where the workload may connect. And the
Readygate doesn't either — it fails closed only until the first config is applied, so an allowlist-free config just makes the proxy ready to pass everything.This PR gives console the model it says it lacks, and populates it from two sources, because neither is sufficient alone:
Design notes
Derived from granted credentials, not the served subset.
served_credentialsdrops any secret whose source is currently undeliverable. Deriving from it would let a vault blip silently rewrite the egress boundary and 403 a host that is still perfectly well authorized. Losing a credential should cost the request its credential, never its destination.Off by default. An existing deployment upgrades with its egress behavior and its
config_hashbyte-for-byte unchanged. No proxy re-applies.Omitted is not empty.
rules: []is not "no allowlist" — it's an allowlist that matches nothing and 403s every request, including the one to the model provider. So the key is absent when off, never null and never empty. Turning the allowlist on with an empty base list is refused by a validation, because that's the same blackout arriving by a different road.warnmode serves the rules but tells the proxy to log what it would have blocked rather than blocking it — the audit pass before enforcing. It rides on anallowlist_warnfield that iron-proxy does not read yet; a companion PR toironsh/iron-proxywould carry it throughTransformsFromSync. Until that lands,warnserves rules the proxy will enforce, so it should be left off.Tests
11 new tests covering off/warn/enforce, the union, dedup, method+path scoping surviving the
http_methods→methodstranslation, the undeliverable-source case, and the empty-base-list guard. Full console suite: 1211 runs, 0 failures, 0 errors.