Skip to content

fix(iam): attach the spored grants as a managed policy (#1573) - #1577

Merged
scttfrdmn merged 2 commits into
mainfrom
fix/1573-spored-managed-policy
Nov 10, 2026
Merged

scttfrdmn merged 2 commits into
mainfrom
fix/1573-spored-managed-policy

Conversation

@scttfrdmn

@scttfrdmn scttfrdmn commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1573

What was broken

Since v0.162.0 the versioned spored grants (#1558) were a second inline policy on the shared spored-instance-role. IAM caps a role's inline policies at 10,240 characters together. On an account where the role carries other tenants' inline policies, every launch on the default role failed with LimitExceeded before an instance existed. The deletes of the older copies never ran, because the write came first.

What this does

The grants are now a customer-managed policy, attached to the role:

role policy
spored-instance-role arn:aws:iam::<acct>:policy/spore-host/spored-policy-v<schema>
spawn-instance-* (and a caller-named --iam-role) .../spore-host/spored-baseline-policy-v<schema>
--nested-spawn role .../spore-host/spored-nested-baseline-policy-v<schema>

Naming. The documents vary only by the account (bindPolicyAccount). They don't vary by region (IAM is global, and every Resource covers all regions) or by role. So one policy per kind per schema serves the whole account. The nested baseline gets its own name because it is a different document, and a managed policy name is per account. Inline, it shared spored-baseline-policy because inline names are per role.

#1558's properties still hold. No released CLI touches /spore-host/. IAM grants the union, so an older CLI rewriting its inline name can only add. A newer schema on the role, managed or inline, means this CLI defers and warns.

Exists with a different document. spawn never calls CreatePolicyVersion, so the 5-version cap never comes into play. It never rewrites an existing policy, and never attaches one it didn't write. It compares the document (semantically: URL-decoded, one-element lists folded, string lists sorted). On a mismatch it warns, naming the ARN and how to remove it, and writes its grants inline beside it.

Baseline roles are included. Their inline budget is shared with user input: --iam-policy-file (spawn-custom-policy, any size), the task staging / nested policy (spawn-scoped-policy), and everything on a caller-named --iam-role.

Order. Attach first. Only then delete the superseded inline copies (spored-policy / spored-baseline-policy, and the -v<=schema inline versions v0.162.0/v0.163.0 wrote). Then detach older managed versions, and delete each one if no other role has it attached (IAM's DeleteConflict is that check).

Fallback, so there's no new failure mode. Every failure on the managed path goes back to v0.163.0's inline write, with a warning that names the actions to grant. That covers AccessDenied on any call, the role's attached-policy quota, the account's policy quota, the conflict above, and any other error. If the inline write then hits LimitExceeded, the error lists every inline policy on the role with its size (whitespace not counted), the total against 10,240, and both fixes. It does not print the bare 409.

Leave no trace.

  • pkg/aws.deleteRole (spawn cleanup, spawn pool drain) deletes a spored policy it detaches when that was the last attachment.
  • spawn cleanup --instance-roles also deletes spored-named policies at /spore-host/ with AttachmentCount == 0 that are more than 24 h old (instancerole.SweepSporedPolicies).
  • A launch whose policy was swept between GetPolicy and AttachRolePolicy re-creates it.
  • The TTL reaper detaches the policies from the roles it deletes but does not delete them. That is reaper: delete unattached spored managed policies (/spore-host/spored-*-vN) it detaches #1576: a reaper grant change plus a redeploy, so it isn't in a patch.

New IAM actions (CLI)

On arn:aws:iam::*:policy/spore-host/*: iam:GetPolicy, iam:GetPolicyVersion, iam:CreatePolicy, iam:DeletePolicy.
On the spawn roles: iam:ListAttachedRolePolicies, iam:DetachRolePolicy (conditionable on iam:PolicyARN), iam:GetRolePolicy (only for the size listing). iam:AttachRolePolicy was already required, for SSM.
spawn cleanup --instance-roles: iam:ListPolicies (*).
docs/reference/cli-iam-actions.md is regenerated. The documented least-privilege baseline lives in spore-host/spore-host (docs/reference/iam-permissions.md): companion PR spore-host/spore-host#691. The portal onboarding template only passes spored-instance-role and never writes IAM, so it doesn't change.

IAM call sequence (for live verification)

Before every sequence: sts:GetCallerIdentity, then ListRolePolicies(role) and ListAttachedRolePolicies(role).

  1. First launch on an account (role carries the legacy inline spored-policy, maybe inline spored-policy-v1, plus others' policies): GetPolicy(P) → NoSuchEntity → CreatePolicy(Path=/spore-host/, Name=spored-policy-v1) → AttachRolePolicy(role, P) → DeleteRolePolicy(role, spored-policy) → DeleteRolePolicy(role, spored-policy-v1). No PutRolePolicy. If AttachRolePolicy answers NoSuchEntity (IAM eventual consistency right after CreatePolicy), it is retried for up to 5 s (0.5, 1, 1.5, 2 s). After that, spawn re-checks with GetPolicy, re-creating the policy once if it is gone, before falling back to inline.
  2. Steady state: GetPolicy(P) → GetPolicyVersion(P, v1) (compare). Nothing written.
  3. Another role of the same kind: GetPolicy(P) → GetPolicyVersion(P, v1) → AttachRolePolicy(role2, P).
  4. CreatePolicy (or GetPolicy / AttachRolePolicy / ListAttachedRolePolicies) AccessDenied: a warning, then v0.163.0's inline write: PutRolePolicy(role, spored-policy-v1) → DeleteRolePolicy(role, spored-policy).
  5. Case 4 with a full inline budget: PutRolePolicy → LimitExceeded → GetRolePolicy for each inline policy (for sizes) → an actionable error.
  6. Attach LimitExceeded (10 attached): a warning listing them, then case 4's inline write.
  7. Newer schema on the role (managed spored-policy-v2 or inline spored-policy-v2): a warning, nothing else.
  8. Schema bump, v1 attached: GetPolicy(P2) → CreatePolicy(P2) → AttachRolePolicy(role, P2) → (inline deletes) → DetachRolePolicy(role, P1) → DeletePolicy(P1) (DeleteConflict = still used elsewhere, kept).
  9. Policy exists with a different document: GetPolicy → GetPolicyVersion (mismatch) → a warning, then case 4's inline write. Nothing is attached or rewritten.

The emulator tests run 1, 2, the mixed-version cases and the deleter. The fake runs every case.

Tests

New, in pkg/aws/iam_spored_managed_test.go. The fake enforces the inline cap, the 6,144 managed cap, the 10-attachment quota and DeleteConflict, and checks the role's grants after every call:

  • the issue's account (13 inline policies, 7,676 chars, legacy 2,775) succeeds with no inline write, and the legacy copy is removed afterwards;
  • AccessDenied on each managed call falls back to inline;
  • inline LimitExceeded produces the size listing and both fixes;
  • a newer schema, managed or inline, means defer;
  • no window without grants, across five transitions including a schema bump;
  • a schema bump retires v1, keeping it when another role still has it;
  • an existing policy with a different document is left alone;
  • the attached-policy quota and an unexpected error both fall back;
  • a policy swept before attach is re-created;
  • v0.161.0 / v0.162.0 / this CLI, mixed on one role;
  • the documents fit 6,144 (2,806 / 2,104 / 2,139);
  • names and partition;
  • the deleter.

Also: pkg/instancerole/sporedpolicies_test.go (the sweep rule), a cmd test for cleanup --instance-roles, and an emulator test that the baseline policy is deleted with its last role and never with spored-instance-role's. TestSporedRolesFitTheInlineLimitWhenMixed now runs every role kind in both managed and fallback modes. The emulator tests that read spored-policy-v1 inline now read the attached managed policy.

make check steps all pass locally. A few fail only because the scratch path contains a space (/Volumes/External HD): hermetic-exec in the Makefile, dashboard-api's TestSetupAllRunsEveryStepFromAnyDirectory, and the check-build-tags file matcher. I ran the hermetic suite by hand and it is green. semgrep scan --config=auto --error on the changed files is clean.

Not merged: the owner verifies live first.

Since v0.162.0 the versioned spored grants (#1558) were a second inline
policy on the shared spored-instance-role. IAM caps a role's inline
policies at 10,240 characters together, so on an account where the role
carries other tenants' inline policies every default-role launch failed
with LimitExceeded before an instance existed.

The grants are now a customer-managed policy at /spore-host/, one per
document per schema per account (spored-policy-v1,
spored-baseline-policy-v1, spored-nested-baseline-policy-v1), attached to
the role. It does not count against the inline budget. #1558's properties
hold: no released CLI touches it, a newer schema on the role (managed or
inline) is deferred to, and an existing policy is never rewritten; one
with a different document is left alone and the grants go inline beside
it. Superseded inline copies and older managed versions are removed only
after the policy is attached.

Every failure on the managed path (AccessDenied, quotas, anything else)
falls back to the inline write v0.163.0 did; an inline LimitExceeded now
lists the role's inline policies with their sizes and names both fixes.

Deleters: deleteRole (spawn cleanup, pool drain) deletes a spored policy
it detaches from its last role; spawn cleanup --instance-roles deletes
unattached spored policies a day old.
…cy (#1573)

A policy created a moment ago can be NoSuchEntity to AttachRolePolicy.
Falling back to inline then would fail the launch on exactly the role
#1573 is about, one with no inline room, so retry the attach for up to
5 s first.
@codecov

codecov Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.19048% with 58 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/aws/iam_spored_managed.go 86.28% 38 Missing ⚠️
cmd/cleanup.go 76.11% 16 Missing ⚠️
pkg/instancerole/sporedpolicies.go 93.47% 3 Missing ⚠️
pkg/aws/iam.go 94.73% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@scttfrdmn

Copy link
Copy Markdown
Contributor Author

Live check, spore-host-dev (435415984226) us-east-1, 2026-11-10

The setup is a throwaway caller-named role s1573-throwaway, plus an --iam-policy-file of 8,510 bytes minified, so the inline baseline can't fit. This exercises the baseline variant of the managed-policy path. The shared spored-instance-role variant wasn't padded live, because changing a shared role is outside what this session may do. That variant runs through the same code, and the unit tests cover it with the issue's exact 13-policy account.

Build Result CloudTrail
v0.163.0 (Homebrew) ❌ PutRolePolicy spored-baseline-policy-v1 … LimitExceeded, no instance CreateRole → PutRolePolicy custom → AttachRolePolicy SSM → PutRolePolicy baseline-v1 LimitExceededException
this PR (2a2125e) ✅ i-00125da3cd4ee5a20 t4g.nano running PutRolePolicy custom → AttachRolePolicy SSM → CreatePolicy /spore-host/spored-baseline-policy-v1 → AttachRolePolicy. No inline baseline write.
  • The role ended with inline spawn-custom-policy only, plus the managed baseline and SSM attached.
  • spored worked on its grants from the managed policy: spawn:dns-status=registered and spawn:last-heartbeat were written (spored 0.163.0).

Teardown and leak check (independent):

  • Instance terminated.
  • Role, instance profile, inline policy and attachments deleted.
  • /spore-host/spored-baseline-policy-v1 deleted (0 attachments).
  • Nothing left: no spawn-managed instances in us-east-1/us-west-2, no EIPs, unattached volumes, non-default SGs, s1573 roles/profiles or /spore-host/ policies. The DNS record is gone.
  • Key pairs are the 3 pre-existing ones.
  • spored-instance-role is untouched.

Not exercised live:

  • the AccessDenied → inline fallback, and its LimitExceeded message (unit-tested only);
  • steady-state re-launch;
  • a schema bump.

@scttfrdmn
scttfrdmn merged commit c2bdd0a into main Nov 10, 2026
12 of 13 checks passed
@scttfrdmn scttfrdmn mentioned this pull request Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant