Repository navigation
feat(sandbox): grant a GCP sandbox read on the registry it pulls from - #663
Conversation
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Two fixed in 49a4fb2, one is real and tracked but out of scope here. Cleanup Skips Live Grants — fixed, and it was my regression. You are right, and the comment six lines above the guard already said so: the marker is written after the cloud grant lands, so a false one still covers a live grant. My early return would have left registry read alive after the deployment was deleted. Reverted to failing loudly. For context on what I was trying to fix: counting a GCP sandbox widened a prefix-blind guard, so a GCP sandbox whose image Alien never hosted now reaches the binding lookup and errors on a manager that hosts no images. That behaviour is pre-existing on AWS, where sandboxes were always counted, so GCP now simply matches it. Erroring loudly beats silently dropping a revoke. Sandbox Permissions Decode Incorrectly — fixed. The read path now derives its service agents from the same table the write path uses, so a member decodes as the type that produced it. Added Existing Grants Stay Incomplete — real, tracked, not fixed here. Worth knowing alongside it: this binding only matters cross-project. Google grants the sandbox agent a project-scoped role that already carries |
|
@greptile review |
|
Both valid. Neither is fixed here, and here is why. Issue 1 — the distinction you are asking for cannot be made at this call site. You are right that the path should tell an unpersisted partial grant apart from a deployment that could never have been granted. It cannot, because That leaves two wrong answers, and I have now written both. Returning I am keeping the loud one. A cleanup that retries is recoverable; a grant that outlives its deployment is not. The real fix is to stop discarding the reason one function up, which changes that signature for every caller, so it is filed separately rather than attempted as a third version of this guard inside a feature PR. Issue 2 — real, filed, and broader than this diff. One thing worth knowing about the blast radius of both: this binding only matters cross-project. Google grants the sandbox agent a project-scoped role that already carries |
|
Ordering note: this waits on #675, which is at 5/5 and touches the same code in #675 gives Once #675 merges I'll rebase this branch onto it, port the per-platform sandbox question onto the new signatures, and fix the two findings here:
|
A GCP sandbox runs a container it pulls from the registry Alien hosts, but the grant counted a sandbox only on AWS, so the image it needs was never covered. Which field names that image now follows the platform. AWS builds a MicroVM image from a published bundle, so `code.image` is the bundle and `privateBaseImage` is what the build pulls; GCP pulls `code.image` itself; Azure names an image from its own catalog and reads nothing of ours. Each compute service is granted as its own Google-managed service agent, since a sandbox pulls as a different principal from a worker and a grant made for one reaches neither the other. The same list is revoked, because a member the revoke does not name stays on the repository policy with nothing left to remove it. The grant also moves off the deployment's extra service-account emails, which are revoked per deployment: the first deployment deleted in a project would otherwise strip a binding its siblings still need. The service-type list is refcounted and survives until the last consumer goes.
|
Rebased onto main now that #675 has landed, and squashed to one commit. The two designs had to merge rather than stack: #675 made a sandbox's registry reference alien: manager 247, bindings 339, @greptile review |
49a4fb2 to
c8801bf
Compare
This comment has been minimized.
This comment has been minimized.
Three gaps found in review, all on the AWS grant path. A China registry answers on `.amazonaws.com.cn`. The parser stripped only `.amazonaws.com`, leaving `.cn` on the region, so a valid image in a `cn-*` region was refused and its grant never completed. Cleanup read the stack to name the repository it must revoke, so a release that removed the sandbox entirely left the grant with nothing naming it and the delete failed forever. A sandbox is only ever granted its own project's repository, so a recorded grant now revokes that repository without reading the stack. A deployment that never recorded a grant no longer fails its delete when no registry binding can be loaded: the grant would have been made through that same binding, so there is nothing to revoke and erroring only parks the deployment at `deleting`.
…licy read honest A cross-account grant is named by the project, not by the deployment that asked for it, so the last deployment leaving a GCP project has to remove every principal the project was granted — a worker-only deployment was removing only Cloud Run's agent and leaving the sandbox agent reading Alien's registry with nothing left to revoke it. The grant and revoke paths also read an IAM policy before writing it back. Reading any failure there as "no policy" wrote a policy holding one deployment's members, with no etag to lose the race on, over one every deployment shares. Only a repository that is gone answers that question now. Alongside those: the delete revokes the union of what the record names and what the stack derives, so a grant written before its state was saved is still removed; a GCP grant that names no repository falls back to the registry prefix, since GAR binds its policy there; and an empty project number no longer completes a grant that reached no compute agent.
|
@greptile review |
… will not answer A delete that cannot reach the registry has to keep failing, since the IAM write lands before the state recording it and this is the last chance to revoke. A manager that never had a registry binding is the opposite case: nothing was ever granted through one, and failing forever would strand the deployment at `deleting`. The probe now reports which it found, reading `BindingNotConfigured` as the contract for "no such binding". Without that split, a sandbox on a public image under a manager with no registry could never finish deleting.
|
@greptile review |
1 participant
Summary
A GCP sandbox runs a container it pulls from the registry Alien hosts, but the
grant machinery counted a sandbox only on AWS, so the image it needs was never
covered. This grants it, and makes the service types the grant and the revoke
name one definition instead of two.
When a deployment reconciles its registry access:
registry at all. AWS builds a MicroVM image from a published bundle and GCP
pulls a container, so both earn a grant; Azure names an image from its own
catalog and earns nothing.
A sandbox pulls as a different principal from a worker, so a grant made for
one reaches neither the other.
the revoke does not name stays on the repository policy with nothing left to
remove it.
This changes a hardcoded "only AWS sandboxes pull from us" to a question asked
per platform.
What I did
Replaced the
INCLUDE_SANDBOX/EXCLUDE_SANDBOXconstant pair with a functionthat answers per platform, and added a sandbox compute-service type mapped to the
Vertex sandbox service agent.
Moved the grant off the deployment's extra service-account emails, which was the
wrong place: those are revoked per deployment, so the first deployment deleted in
a project would strip a binding its siblings still need. The service-type list is
refcounted and survives until the last consumer goes.
Fixed a regression the widened guard introduced. That guard is deliberately
prefix-blind, so it now admits a GCP sandbox whose image Alien never hosted. Such
a deployment was never granted, and on a manager that hosts no images there is no
registry binding to load, so its delete reconcile started returning an error every
tick. It now answers that there is nothing to strand.
How I tested
cargo test -p alien-manager -p alien-bindings --lib— 239 and 338 pass, andcargo check --workspace --all-targetsis clean, on a branch rebased onto currentmain.
Pinned the new behaviour rather than assuming it: a GCP sandbox-only stack is
covered by the grant, the shared project grant survives while any sibling
deployment is alive, and a never-granted GCP sandbox on a public image reaches
the binding lookup with nothing recorded to clean up.
Checked the attacks this change makes possible:
diff: GCP has one registry repository per environment, so the worker agent
already holds this. Tracked separately; this adds a second principal inside the
same customer project, which holds no capability the customer lacks today.
removal is a
retain, so naming one never granted is a no-op while omitting onewould orphan a member (
gar.rs:207).members are revoked only in the last-consumer branch, and the per-deployment
identity stays per-deployment (
registry_access.rs:92-152).created at a project's first session, so the binding necessarily precedes it.
The binding is accepted; whether it becomes effective when the agent appears is
measured by the cross-project pull test, not claimed here.
Nothing turned up.