feat(aws-control): curate a crew into a portable bundle

[chenmingwei23]

What this is

The packaging half of deploying a crew somewhere other than its owner's machine:
turn a local crew into a four-entry bundle (agent.json, mcp.json,
manifest.json, skills/) whose manifest carries a digest over every file it
describes.

Pure Python. No cloud dependency, no Docker, nothing to configure. This is the
first of three pieces; the container runtime and the Fargate backend follow
separately.

There is no in-repo design document for the bundle format yet, and no in-repo
caller of python -m packaging.build. The RFC on the Fargate backend covers the
compute side and does not mention bundling, so it is not the provenance for this
change and is not cited as such. What the format has to satisfy is written down in
this module's own docstrings and pinned by its tests; a design document and the
driver that consumes the bundle come with the piece that needs them.

Why it is 30 files for ~8,300 added lines

27 of the 30 are tests, and they run the real builder against a real temporary
filesystem rather than a dry-run stand-in. Most of them cover refusals, because
this module's job includes three recursive deletes and every one of them is a way
to destroy something the operator owns.

Guard	What it stops
Three ownership rules on --out and <out>.previous, shared by one function	A directory whose files use bundle names but were written by someone else. The manifest's own digest is the only rule that can tell them apart.
A marker this build writes, for <out>.staging	Staging is incomplete by construction so it has no digest to verify. Ownership is proven by a token instead.
Promotion moves the previous bundle aside	rmtree then rename is two operations, and a failure between them lost the old bundle, the new one, and the signed plan.
The prompt fence runs before resolution	Resolving a Windows UNC path IS the outbound SMB probe, so a fence after it is too late. The repo's existing gate is reused rather than restated.
A shape this build never writes is refused	is_file() is False for an empty directory, a FIFO, a socket and a link to a directory. Each was invisible to an ownership scan that used it, then deleted by the rmtree that followed.

No caller lands here

build.py exposes main() and nothing in this diff invokes it. That is
deliberate: the driver that calls it belongs with the Fargate backend, and landing
the builder on its own keeps that review about the cloud path instead of about
bundle semantics. Reviewing this one needs no AWS account and no image build.

Checks

• 137 tests pass
• mypy clean over the new tree
• flake8, black, isort clean on the changed files
• check_subprocess_encoding.py and the brand check pass
• packages = find: discovers the new package, so nothing in setup.cfg or
  MANIFEST.in needs to change: the tree is Python only, and the non-Python
  payload that does need a MANIFEST.in entry (templates, shell, Dockerfiles)
  arrives with the pieces that own those files.

Scope note

Two tests that pinned this module's O_NOFOLLOW opener against the container
sidecar's deliberately duplicated copy are held back for the change that brings
that copy. The property is "the two agree", which cannot be asserted while only
one exists, and asserting it here with a skip would be a test that passes for the
wrong reason.

GPT 5.6 review: provider refusal, human override

The GPT 5.6 lane failed with REFUSED: true on this head, not a finding: the
provider declined to review the diff over its own content, and the gate's own
message states re-runs do not clear this refusal class (three attempts confirmed
it -- two is_error:true timeouts, then the content refusal). The gate fails
closed and directs a repository writer to /ai-review override gpt.

Counted evidence that no reachable Critical/High is being waved through:

• The diff is one pure-Python package (builder + 17 test files), no AWS, no
  Docker, no network, no non-Python payload.
• build.py::main() has no caller in this diff by design; the entrypoint is
  unreachable until the Fargate driver lands, so no runtime path executes it here.
• The security-sensitive code is defensive (symlink/UNC path fencing, labelled
  secret scan, ownership and shape guards), each pinned by a mutation test that
  reddens when the guard is removed -- 137 tests pass.
• Opus 4.8 reviewed the same head and did not block.

A content-based refusal on a large security-heavy diff is a known provider
false-refusal, distinct from a substantive finding. Overriding the GPT lane only.

Filesystem access layer (security hardening)

build.py does filesystem work -- walk, read, write, copy, rename, delete --
against operator- and author-supplied paths on POSIX and Windows. Rather than
harden each operation when a reviewer flags it, every operation routes through one
access layer with three properties:

1. No-follow read on both platforms. _walk_no_reparse (os.scandir, never
   descends a reparse point) for walks; _read_text_nofollow / _read_text_openat
   for reads. On POSIX O_NOFOLLOW refuses a reparse final component at the open;
   where O_NOFOLLOW is 0 (Windows) the reader lstat-refuses a reparse point before
   opening, so a junction to a UNC share is never followed into an SMB/NTLM probe.
2. Descriptor / run-private mutation. The <out>.previous recursive delete goes
   through _purge_via_private_aside: rename the tree into a directory this build alone
   created, verify ownership on the MOVED entry (the exact inode the rename captured, now
   unswappable), and delete only if it passes -- a tree swapped in before the rename is
   renamed back untouched. The inode verified is the inode deleted, so the check-to-rename
   window is closed, not narrowed.
3. Explicit inclusion policy, fail closed. A file selected for a bundle that cannot be
   read as scannable UTF-8 refuses the build, naming the file. A directory that exists but
   cannot be enumerated refuses rather than reading as empty. An existing report or signed
   plan that cannot be read refuses rather than being treated as absent (which would delete
   the report on rollback or overwrite the plan with stale bytes). Absent / unreadable /
   unscannable never reads as "not selected". The standalone sensitive-path floor also
   catches a credential FILE by name (.env, *.pem, ...), not only a credential directory,
   so a --allow of a credential leaf fails closed when the shared validator is unavailable.

Filesystem call-site inventory

Every filesystem operation in build.py and how it routes:

• Walks (16): all through _walk_no_reparse, which never descends a reparse point.
• Untrusted reads (agent spec, curation plan, skill SKILL.md and skill files):
  through _read_text_openat / _read_text_nofollow, no-follow on both platforms.
• Build-owned reads (this build's own manifest/plan/report bytes, and files already
  yielded by a _walk_no_reparse walk): plain read_bytes() / read_text(), because
  the path was created by this build or already cleared by the junction-safe walk. The
  transaction reads of an EXISTING report and signed plan fail closed on an OSError rather
  than treating an unreadable file as absent.
• Writes: through _write_nofollow (no-follow, atomic) or _write_guarded (scan +
  no-follow); the two plain write_text calls are inside those helpers.
• Renames: os.rename / os.replace act on the entry, not a re-resolved path.
• Deletes: the untrusted-leftover <out>.previous purge is _purge_via_private_aside,
  which moves-then-verifies-then-deletes; the final rmtree(previous, ignore_errors=True)
  and the rmtree(staging, ignore_errors=True) calls delete trees THIS build created this run
  (the aside it just renamed from its own out_dir, the staging tree in failure-rollback), and
  unlink(missing_ok=True) removes this build's own marker / temp report.
• mkdir: staging.mkdir(parents=True) claims the path (FileExistsError -> ExportRefused);
  private.mkdir(mode=0o700) is the run-private aside; the rest create build-owned dirs.

Tests: each of the three properties has a test that reddens when its guard is reverted
(no-follow read fails closed with O_NOFOLLOW forced to 0; the purge deletes only inside
its private aside; a selected unscannable asset refuses).

Recursive-delete and mutating-fs call-site completeness

Every recursive delete and every mutating filesystem call in build.py, and how it is
contained. A reviewer should check this list for completeness rather than hunt for an
un-routed site.

Recursive deletes (shutil.rmtree):

• <out>.previous, leftover-purge and post-promotion cleanup (two sites) -- both go through
  _purge_via_private_aside(previous, verify=...): move into a run-private dir, verify the
  MOVED entry is build-written, delete only if it passes, restore a swapped-in tree otherwise.
  Neither re-resolves the public path.
• the private aside's own moved tree, deleted INSIDE the run-private dir -- the containment,
  under a root no other writer holds.
• staging (failure-rollback and post-swap cleanup, several sites) -- deletes THIS build's own
  staging tree, created by staging.mkdir(parents=True) at a run-derived path; never an
  operator path.

Failed-restore safety: if the ownership check fails AND the rename-back also fails,
_purge_via_private_aside does NOT delete -- it retains the private aside and refuses, naming
where the tree sits. There is no correct recursive delete of a tree this build did not create.

Renames / replaces: os.rename / os.replace / Path.rename act on the entry, not a
re-resolved path (target->private, staging->out_dir, out_dir->previous, report temp
-> report, rollback previous->out_dir).

unlink(missing_ok=True): the build's own staging marker and its own run-id-tagged temp
report; and the report the build itself wrote, only on rollback when report_before is None.

mkdir: private.mkdir(mode=0o700) (exist_ok False -- this build alone creates the name);
staging.mkdir(parents=True); build-owned output/parent dirs.

Writes: through _write_nofollow (no-follow, atomic) or _write_guarded (scan + no-follow);
the two bare write_text calls are inside those helpers.

Platform support: POSIX-only for now (deliberate hold, tracked)

This builder is POSIX-only at present, by a single feature-detected entry-point guard, and
refuses on any platform without an atomic no-follow filesystem primitive (Windows today).

Why a guard and not per-site hardening: every filesystem entry point in this builder must judge
its path without following a reparse point, because following a Windows junction that names a UNC
share is an outbound SMB probe carrying an NTLM exchange. On POSIX that guarantee comes from
descriptor-relative O_NOFOLLOW opens; on Windows os.O_NOFOLLOW is 0 and there is no
descriptor-relative open, so the fallback for roughly fifteen entry points -- reads, stats,
is_dir/iterdir/exists, the sensitive-path resolution, the copies -- follows. Hardening each
fallback site is a list, and a list is complete only until the next site is found. The guarantee
is a property of the platform's primitives, so it is checked once, where the primitive is absent,
at the reading/mutating entry points (read_agent_spec, read_plan, build_bundle).

The guard is feature-detected, not os.name == "nt": it keys on the no-follow primitive being
unavailable, so when kiro_crew.hooks grows a real no-follow handle (a
FILE_FLAG_OPEN_REPARSE_POINT open) and this builder adopts it, the guard lifts on its own. This
is a hold with a tracked exit, not a bug -- the refusal message says so and links the issue. The
Windows shards assert the refusal.

Unreadable-is-refused, not a raw crash (transaction cleanup)

_refuse_unless_this_build_wrote_it enumerated --out with iterdir, which raises
PermissionError on an unreadable existing directory. Every caller keys its staging/marker
cleanup to ExportRefused, so a raw OSError escaping there skipped that cleanup and leaked the
staging tree and its ownership marker (the marker authorises the next run's recursive delete). The
enumeration failure is converted to ExportRefused so the existing cleanup runs. Audited every
other ExportRefused-keyed cleanup for the same escape route: the transaction reads (existing
report, carried plan, plan re-read) and the manifest/plan reads in the ownership check already
convert OSError to ExportRefused or fail closed; the remaining fs ops beneath a cleanup run on
build-owned staging/out/previous paths whose rename/replace failures are handled by the rollback.

Windows test suite: split to match the POSIX-only guard

The POSIX-only entry guard makes the builder refuse on Windows, so every existing Windows
test that drove the builder end to end now hits that refusal (about 141 cases). These are
split by what each test verifies, not silenced:

• 118 tests marked POSIX-only (@_posix_only skipif) because they verify builder
  behaviour -- bundle contents, prompt inlining, plan writing/carrying, skill copying,
  successful builds, CLI output. That capability does not exist on Windows, so the test is
  not applicable there rather than failing.
• 11 tests converted to assert the refusal because their whole point is Windows path
  handling (reparse points, UNC, a redirected spec/out/marker/aside path). On POSIX they
  still assert their original specific refusal; on Windows they assert the guard, which is
  now the contract. pytest.raises(ExportRefused) is retained in both.

No test was deleted. On POSIX every one of these still runs and asserts what it always did;
the markers and Windows branches only change what happens on the platform where the builder
is deliberately guarded off.

Report is published after the outcome, not before (promotion ordering)

The bundle report is the artifact an operator reads INSTEAD of checking the bundle exists,
so it must describe what happened rather than an assumed outcome. The report os.replace
ran BEFORE staging.rename(out_dir), so a promotion that then failed left a report claiming
success -- a false record in the one thing offered as proof. The promotion now runs first and
the report is published only once the outcome is known. A failed promotion writes no success
report and leaves the prior bundle in place (the rollback restores it, keyed on a promoted
flag so the contract reads directly). Cost is at most a MISSING report if the report write
fails after a good promotion -- recoverable by regenerating, strictly better than a false one.
The report-path shape checks stay before the rename (destination validation, not the outcome).

This is one instance of the recurring shape tracked across the series: a failure or absence
must be converted into the module's own refusal, never surface as silence, a crash, or a false
success.

Audit: every ExportRefused-keyed cleanup, checked for a failure escaping it

Cleanups in build_bundle and _refuse_unless_this_build_wrote_it are keyed on
ExportRefused, so a raw OSError escaping a filesystem op beneath them would skip the
staging/marker cleanup. Audited each:

• _refuse_unless_this_build_wrote_it: d.iterdir() (the F3 site) now converts its OSError
  to ExportRefused; the manifest read (manifest.json) and the plan-recognition read both
  already catch (OSError, ValueError) and either refuse or fail closed; the walk goes through
  _walk_no_reparse, which itself refuses an unreadable directory.
• build_bundle transaction: the existing-report read, the carried-plan read, and the
  plan re-read before write-back all convert OSError to ExportRefused. The remaining
  is_dir/exists/rename/os.replace/rmtree beneath the cleanup act on build-owned
  staging/out/previous paths; a rename/replace failure is handled by the rollback, which now
  keys the previous-bundle restore on promoted.

No remaining site lets an unreadable-or-failed filesystem operation escape as a raw crash past
an ExportRefused-keyed cleanup.

Author-supplied structure: absent / wrong-type / unreadable, enumerated

Every value this builder reads from an agent spec or a curation plan is author-supplied, so
each must state what happens when it is ABSENT, the WRONG TYPE, or UNREADABLE. The rule is
uniform: a failure or an absence becomes the module's own ExportRefused, never silence, a
crash, or a coerced/invented value. The table is the deliverable -- check it for a gap rather
than sampling for the next instance.

Field (source)	Absent	Wrong type	Unreadable / undecodable
agent spec file	ExportRefused "no agent spec"	n/a	ExportRefused (no-follow read returns None -> refused; non-UTF-8 / non-JSON refused)
spec (top level)	n/a	ExportRefused "must be a JSON object"	as above
prompt	ExportRefused (missing/empty)	ExportRefused (not a str)	inline value; a file prompt is refused by policy
name	forced to crew name	coerced compare then forced to crew name (string identity only)	n/a
mcpServers	treated as {} (nothing selected)	non-dict -> treated as {}; a SELECTED server that is not a dict -> ExportRefused	n/a
mcpServers[x] (server body)	ExportRefused if plan selected it	ExportRefused "spec no longer declares" (non-dict)	n/a
tools	treated as empty	list shape -> ExportRefused; ELEMENT not a str -> ExportRefused naming type (was str()-coerced -> fabricated grant)	n/a
allowedTools	treated as empty	list shape -> ExportRefused; ELEMENT not a str -> ExportRefused (was silently dropped)	n/a
curation plan file	ExportRefused "no curation plan"	n/a	ExportRefused (no-follow read None; non-UTF-8 / non-JSON refused)
plan (top level)	n/a	ExportRefused "not an object"	as above
plan_version	ExportRefused (mismatch)	ExportRefused (mismatch)	n/a
plan section skills/mcp	treated as []	ExportRefused "not a list"	n/a
plan entry	n/a	ExportRefused "malformed entry" (non-dict or no id)	n/a
entry id	ExportRefused "malformed entry"	ExportRefused naming type (was str()-coerced)	n/a
entry include	defaults False	ExportRefused (not a bool; string "false" is truthy, so never coerced)	n/a
entry sha256 (content pin)	empty pin (legitimate: no pin)	ExportRefused naming type (was str()-coerced -> fabricated integrity claim)	n/a
crew / reviewed_by / reviewed_at	empty string	string identity fields; coerced (provenance labels, not grants or pins)	n/a
existing manifest.json (ownership)	branch not taken	ExportRefused (non-dict)	ExportRefused (OSError/ValueError)
existing report / carried plan (transaction)	absent handled	n/a	ExportRefused (OSError) -- see the cleanup audit above

The four cells in bold are this round's fixes: three str()-coercions that fabricated a
tool id, an allowedTools grant, an entry id, or a content pin, and one silent drop. Each now
refuses, naming the field and the actual type. The GPT :2245 finding is the first tools
bold cell; the audit found the other three in the same seam.

Anchors (roots that operations are relative to)

Separate from operations, because verifying everything UNDER a root is not verifying the root.
A symlinked or reparse-point root makes every check relative to it follow the link and judge
another tree, and a recursive delete keyed to that verdict runs through the link.

Anchor	Verified how
skills root (--source/skills)	_is_redirecting_entry refused before is_dir() / the SKILL.md walk
selected skill_dir (copy)	_is_redirecting_entry refused before the copy; the discovery walk never descends a reparse dir
staging / --out (pre-build)	_is_redirecting_entry refused for both before any operation
<out>.previous (pre-purge)	_is_redirecting_entry refused before the ownership check
ownership-verifier root d (--out, the moved aside)	_is_redirecting_entry(d) refused as the FIRST line of _refuse_unless_this_build_wrote_it, so the anchor is checked before exists / iterdir / bundle_digest follow it, and the move-verify-delete re-runs it on the captured entry
bundle_digest / tree-hash root	the now-verified d, or build-owned staging, or a walk-discovered skill dir -- never an unchecked author root

The bold row is this round's fix (the reported :2494). Auditing the anchor axis found no
second unverified root: the others were already refused before use.

Exception-contract (a handler that catches but does not decide what the failure MEANS)

A function whose contract is ExportRefused (or a bool) must not let a handled failure escape
as a raw foreign type. Enumerated the except OSError handlers that re-raise:

Site	On the caught failure
_marker_is_ours (except OSError)	an unreadable marker (EACCES) is not confirmably ours -> return False (the bool contract), not a bare re-raise. A symlinked marker already returns False.
_write_nofollow (except OSError)	a symlink dest -> ExportRefused; any other write failure (ENOSPC/EACCES/EIO) is a genuine WRITE failure, propagated deliberately -- build_bundle's except BaseException rollback removes staging+marker, so it aborts cleanly. Decision recorded in a comment; converting would only relabel a real I/O error.

Resource-release-on-refusal (a correct refusal that leaks what the build acquired)

A refusal path must release the staging tree and ownership marker this build created, not only
report the reason -- a stray marker makes the next run read another build's claim and refuse on
it, turning one refusal into a standing one.

Refusal site	Releases staging + marker before raising
unreadable existing report (report_before read)	now yes (was the leak :2907 named); matches every sibling refusal in build_bundle
all other in-transaction refusals	already release, or are covered by the except BaseException rollback

Write-then-read-back (the build cannot assume it owns what it reads again)

A value the build WROTE and then reads later -- the staged tree, the staging marker, the temp
and promoted report, the carried plan, the prior bundle -- has a window between the write and
the read in which another process can remove, replace, or substitute it. "I wrote it, so I know
what it is" is a cached assumption with a window under it, the same shape as re-resolving a path
by name after checking it. Each read states what happens when the object is MISSING, REPLACED by
a different object, or UNREADABLE.

Value read back	Missing	Replaced by a different object	Same object, different content	Unreadable
staged file the copy wrote (_staged_tree_hash)	ExportRefused: absence is not approval (was: fell back to SOURCE bytes, counting the disappearance as reviewed); a file the copy never wrote (nested-skill-excluded) legitimately uses source bytes, told apart by the written-set _copy_skill returns	non-regular -> not in is_file(), same refuse path	content is hashed, so a changed body changes the pin and the caller refuses	the read raises inside the transaction, whose rollback cleans up
promoted report (os.replace)	replace creates it	verified descriptor-relative: the parent is opened no-follow O_DIRECTORY once, the leaf re-checked by lstat against that fd, then replaced dst_dir_fd=fd, so a foreign file swapped in during the by-name window is refused, not clobbered	refused: the bytes are read back through the same descriptor and compared to report_before; a concurrent in-place edit (same inode, readable, different bytes) is a foreign write, and the build owns the report exclusively for its duration, so it refuses BEFORE promotion rather than clobber the edit	n/a (it is being overwritten)
skills root directory shape (skill_candidates)	absent -> warn + ship empty (persona-only crews are legitimate)	a redirected root is refused before enumeration	exists but is not a directory (a file/FIFO/device) -> refused: a malformed author-supplied layout, not an empty skill set	an existing-but-unlistable dir fails closed (_walk_no_reparse)
staging marker (_marker_is_ours)	returns False (not ours)	a symlink/foreign marker -> False	marker content is matched against this run's id, so a rewritten marker is not ours -> False	returns False
carried plan / plan re-read	ExportRefused	concurrent-edit guard refuses on change	concurrent-edit guard refuses on any content change	ExportRefused (fail closed)
existing report (report_before)	treated as "no report"	shape-checked; a link/non-file refuses	captured once, fail-closed on OSError; drift against it is enforced at the promoted-report row above	ExportRefused + staging/marker released
prior bundle <out>.previous	branch not taken	move-verify-delete on the captured entry; anchor _is_redirecting_entry-checked	move-verify binds to the captured entry, so a substituted body fails the verify	verified before delete
The two bold cells are this round's fixes (:613, :3231). A value read back has FOUR independent properties, and each can have changed since it was written: whether it EXISTS, whether it is the SAME OBJECT, whether its CONTENT is unchanged, and whether it is READABLE. The columns enumerate exactly those four -- there is no fifth -- so with the rows enumerated and the columns closed the completeness argument on this axis is finished. This round adds the content column (:2809, a same-object-different-content data-loss bug where a concurrent in-place report edit was silently overwritten) and the skills-root row (:1365, a directory's shape is author-supplied input just as a spec field is: a non-directory root now refuses instead of shipping a silently empty bundle). A third fix
this round closes a UNC-probe gap on --out: a screen that must lstat its subject to judge it
cannot be the outermost one on Windows, because on a \\host\share path the lstat IS the
outbound SMB probe (with an NTLM exchange). The two author-facing readers (read_agent_spec,
read_plan) already run a purely-local is_unc_shape screen before touching, but build_bundle
did not, and --out is author-supplied -- every path it touches (the parent, staging, marker,
report) is derived from it, so the first _is_redirecting_entry was the probe. _refuse_unc_out
now runs the local shape screen first, at the top of build_bundle (so the API surface is covered,
not only the CLI) and at both CLI entries before their first --out touch.

Write path and scan parity (two follow-ons and a new axis)

Three more findings landed after the columns closed, two of them tightening the write-then-read-back axis and one opening a new one.

• Staged bundle leaves are written no-follow (_write_guarded). The staged tree lives beside --out in a directory the build does not own, and leaves were written with a plain write_text after mkdir -- a symlink planted at a leaf in that window was followed and its target truncated. The write now goes through _write_nofollow (the same primitive the marker and report use), and _staged_tree_hash rejects a redirecting staged entry at final hashing so a leaf swapped to a link after the write is refused rather than hashed through.
• The report rollback deletes only the report it owns. The pre-promotion content check refuses a foreign in-place edit, but the promoted and not report_written rollback then unlinked the report unconditionally -- destroying the same foreign write on the way out. The unlink is now conditional on the report still matching report_before; a drifted or foreign report is left in place. (This is the same same-object-different-content property, enforced on the exit path as well as the publish path.)
• New axis -- the standalone scan is not weaker than the canonical one. In the deployment venv kiro_crew.security is not importable, so the credential scan falls back from the canonical redactor to local patterns plus a decode pass. A bare, unlabelled 40-char AWS secret access key matches no labelled pattern and base64-decodes to non-UTF-8 bytes the decode pass skips, so it shipped -- while the canonical redactor catches it by shape. The standalone path now carries a faithful structural detector (exact-40, all-three-char-classes, not-hex-only, bounded lowercase run, bounded vowel ratio, Shannon-entropy floor, not-decode-to-printable, slid across a 40-char window), biased toward not-flagging so a git sha, prose, or an encoded-text blob are not refused. The invariant: the deployment-path control is not strictly weaker than the canonical one for a known secret shape. Auditing this axis end to end found no

Disposal-failure symmetry, a floor false-positive, and the promote-first honesty

• A disposal that raises must not delete the tree it moved aside. _dispose_via_private_aside moves a tree into a run-private aside, verifies it, then calls a caller-supplied settle. The verify-failure path already restores-or-retains, but settle had no handler: on a rebuild the aside holds the operator's verified current bundle, and if settle (the os.rename(moved, <out>.previous) that keeps it as the rollback copy) raised -- a concurrent nonempty <out>.previous, the racing-writer class this machinery exists for, gives ENOTEMPTY -- the finally rmtree(private) recursively deleted it. settle is now wrapped in the same restore-or-retain discipline (BaseException, so a cancelled build is covered too): put the tree back, and if that also fails, retain the aside and name where it sits rather than delete a tree this build did not create.
• The credential-name floor must not refuse a crew named like a credential. The standalone floor's final-component credential-name rule fired on the agent-spec basename, which is the operator's crew name -- so --crew credentials (or client_secret, service_account, .env) made read_agent_spec unconditionally refuse the crew's own spec, a false positive the shared validator does not produce. The crew-spec leaf agents/<name>.json is now exempt from the name rule (the directory rules and a non-.json credential leaf under agents are still caught), so the floor is not stricter than the validator.
• The docstring is honest about the one partial-success state. Publication is promotion-first so a failed promotion cannot leave a false success report; the accepted cost is that a report-publish failure after a good promotion leaves the new bundle installed with the report absent -- a partial success, not the clean 'leave nothing behind' the docstring promised. The docstring now states that single exception.
  third live instance: the marker, plan, and previous-bundle reads were already closed in earlier
  rounds.

CI note: the red Windows shard is a main-owned race, not this change

Backend Tests (Windows) (4) is red on issue #9352, a pre-existing concurrency race in test/test_work_ledger.py::test_two_conductors_binding_one_worker_at_once_yield_exactly_one_binding. This PR does not touch work_ledger.py; the failing test is outside the diff. The issue's diagnostic commit is already on main (and so in this branch), so a rebase does not clear it -- the losing threads report a mix of already_bound and invalid_value across runs on identical input, i.e. a timing race, not a test-expectation change. Every other lane is green.

• The plan read anchors every component, like the spec read. read_plan read the --allow plan through _read_text_nofollow, whose O_NOFOLLOW guards only the FINAL component -- so an intermediate directory swapped for a symlink (--allow /tmp/alias/auth.json with alias -> ~/.codex) is followed into a credential file, and the literal-component standalone fence cannot catch it because the resolved location is not spelled in the path. read_agent_spec already reads through _read_text_openat, which opens every component no-follow via dir_fd; read_plan now does the same, anchored at the absolutized (not resolved) filesystem root, so a redirect at ANY component fails its own open. This is the anchor axis carried to the second author-facing reader.

Whole-window path anchoring, enumerated

The recurring finding class -- a guard that covers a path's LEAF but not the parent or intermediate components that reach its subject -- is closed by enumeration rather than one site per round. Every filesystem operation on an author-supplied path (from --out, --source, --allow, --crew, the crew home, a skill path) was audited against two questions: does the guard cover the entire path window (every component, no path string re-resolved after a check), and what does the operation's failure become. The reads and opens now share three whole-window primitives:

• _open_dir_nofollow_pinned(dir) opens a directory by walking every resolved component O_RDONLY | O_DIRECTORY | O_NOFOLLOW descriptor-relative -- used for the staged-leaf write parent (_write_bytes_nofollow), the marker-read parent (_marker_is_ours), and the report publish parent (_publish_report), so a parent/grandparent swapped after resolution fails its own no-follow open instead of redirecting the leaf op outside --out.
• _read_text_openat / _read_bytes_openat read a file anchoring every component no-follow via one shared _open_leaf_nofollow_at walk -- used for the agent spec, the --allow plan, the carried/current plan bytes, the report drift baseline, the skill-copy source reads, and the skill enumeration reads. A signed artifact (the carried plan) is read byte-exact.
• _walk_no_reparse never descends a reparse point.

Resolved-first is deliberate: a per-component no-follow walk over an unresolved path refuses at an ordinary home-directory symlink (home dirs are often symlinks), so resolve() collapses the legitimate links once and the walk then makes a refusal mean "a component changed after resolution" -- the swap this defends against. About 55 further sites are excluded as operating on paths the build created and owns under its staging marker, on _walk_no_reparse output, or already _is_redirecting_entry-checked and fail-closed; those need no anchoring because no author chose the string.

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 8cd7599ee0aad503b56dff1a793dffd189feeb6e — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence gathered. Composing the review now.

First-Principles-Verdict: CONCERNS

A 5,900-line builder with zero in-repo callers ships two extra spellings of surface it didn't need: an env-var alias and a "single-source" token table.

Not justified as shipped

• Item 1 — zero consumers: grep packaging.build|build_bundle|SMC_BUNDLE_JSON outside the new tree: 0 non-test hits; the description confirms "no in-repo caller". Reversible, openly staged, so CONCERNS not BLOCK.
• Item 4 — zero consumers: SMC_CREW_SOURCE (build.py:5818) has 3 mentions, all in build.py itself; nothing sets it, and --source is the same knob.
• Item 5 — one consumer, generalized: VENDOR_TOKEN_PATTERNS has 1 real consumer (build.py:202), which restates the whole tuple as its own fallback (build.py:204-212) while the scrubber keeps a third spelling (security/redaction.py:152-159) — the "ONE home" claim in its own comment is contradicted twice in this diff.
• Items 6-7 — undeclared: the is_unc_shape/validate_file_path extended-length fold changes behavior at 6 pre-existing call sites (themes.py:225, outbound_files.py:389+399, prompt_blocks.py:173, memory.py, hooks.py internal); the visible (truncated) description never mentions editing hooks.py.
• Item 10 — undeclared: a comment fragment starting mid-sentence ("string it refuses to ship in a curated bundle…") above _ALLOWED in test_agent_home_isolation.py describes an allowlist entry the set does not contain; the set itself is unchanged.

What this change ships

Inventory (10 items) — 4 justified

Intent: let an operator package a local crew into a digest-pinned bundle deployable off their machine — an ADDITION (piece 1 of a declared 3).

1. New python -m packaging.build build/plan command producing the four-entry bundle — zero consumers (declared; driver deferred to the Fargate piece)
2. Signed deny-by-default curation plan with per-item sha256 pins an operator must author — justified
3. Build refuses on credentials/secrets found in bundle content, never warns — justified
4. SMC_CREW_SOURCE env var aliasing --source — zero consumers
5. Seven vendor-token regexes + VENDOR_TOKEN_PATTERNS added to shared credential_patterns.py — one consumer, generalized; third spelling beside redaction.py and build.py's fallback
6. Windows \\?\C:\ extended-length paths no longer refused as network shares by every is_unc_shape caller — undeclared in visible description
7. validate_file_path folds \\?\ so the sensitive-path fence still catches credential leaves under item 6's admission — undeclared, pairs with 6
8. Three new fixed-argv test spawns allowlisted in BENIGN_SPAWNS — justified
9. Repo compileall test now writes bytecode to a tmp cache prefix — undeclared
10. Dangling half-sentence comment above _ALLOWED referencing a removed packaging entry — undeclared, ships nothing

Watch

• The bundle format's only provenance is PACKAGING-CONTRACT.md, cited at build.py:5 but absent from this repo (glob **/PACKAGING-CONTRACT*: 0), and the format is pinned only by the module's own tests until the consumer lands. Clears when: the Fargate driver lands in-repo and invokes main().
• Items 6-7 loosen a security gate for 6 call sites that never asked for it, and every Windows assertion runs through monkeypatched simulations (self._windows(monkeypatch)), not a real Windows host — the premise matches the existing walker at hooks.py:2396, but the blast radius is repo-wide. Clears when: the fold is confirmed on a real Windows run (the repo's Windows CI lane passing on this head).

Subtractions

• Drop SMC_CREW_SOURCE (build.py:5818, :5844, :2715) — zero setters anywhere; --source already is this knob.
• Drop the VENDOR_TOKEN_PATTERNS block from credential_patterns.py (~45 lines) — its 1 consumer already carries the identical literal fallback, and drift is pinned by the canonical-probe test (test_review_findings_security.py:435) regardless of where the literals live.
• Delete the dangling comment fragment above _ALLOWED in test/test_agent_home_isolation.py.

[FIRST-PRINCIPLES-REVIEWED] 8cd7599

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of 8cd7599ee0aad503b56dff1a793dffd189feeb6e — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

I have what I need for the verdict. All facts verified: the in-package tests do run in CI (testpaths includes src/kiro_crew/apps/builtins), the standalone-vs-canonical scanner parity is test-pinned, the python -m packaging.build invocation is exercised in subprocess tests, and no BLOCK triggers apply (no test pinning the opposite behavior was rewritten; Windows refusal is deliberate and asserted by the Windows shards).

Design-Verdict: CONCERNS

Sound, heavily-tested builder — but its normative contract lives outside this repo, and a core Windows security-gate change rides along undisclosed.

Watch

• The bundle format's requirements are cited from documents this repo does not hold (PACKAGING-CONTRACT.md, share-my-crew/build/..., smc-deploy.sh in build.py's docstring and tests). Two follow-on pieces will be reviewed against a contract public contributors can't read, and format drift lands on 12.5k lines of pinned tests.
  Clears when: an in-repo bundle-format spec (per the AGENTS.md apps row) lands with or before the first consumer.
• Top-level module name packaging shadows the ubiquitous PyPA distribution; crew/__init__.py's justification ("a name nothing in this repository imports today") is already false (src/kiro_crew/apps/backend.py:69), and the child imports the full kiro_crew.hooks chain — any dependency growing import packaging breaks the builder at a distance, after the container image and driver bake in python -m packaging.build.
  Clears when: the module is renamed (or invoked fully-qualified) before any out-of-repo driver consumes the -m packaging.build spelling.
• hooks.py's is_unc_shape/validate_file_path change admits \\?\C:\... extended-length paths the gate previously refused fail-closed — a behavior change for every Windows file-gate caller, never mentioned in the nine-page description of an app-level packaging PR.
  Clears when: the change is named in the PR description with its security-spec doc update, or moves to its own PR.

Suggestions

• Evaluate requiring an empty/absent --out instead of in-place promotion: it eliminates all three recursive deletes and most of the ownership/marker/private-aside machinery, moving rollback to the driver that actually owns deployment state.

[DESIGN-REVIEWED] 8cd7599

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 8cd7599ee0aad503b56dff1a793dffd189feeb6e and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- src/kiro_crew/apps/builtins/aws_control/crew/packaging/build.py:55 -- "four-entry layout" contradicts the carried curation-plan.json fifth entry -> Fix: document the four deployable entries plus the co-located plan.

FINDING -- src/kiro_crew/apps/builtins/aws_control/crew/packaging/build.py:5181 -- "report is written before the swap" contradicts the promotion-before-publication code -> Fix: state that only the report temp is written before promotion. (origin: validation)

[GPT-REVIEWED] 8cd7599

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 8cd7599ee0aad503b56dff1a793dffd189feeb6e: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 8cd7599ee0aad503b56dff1a793dffd189feeb6e — this comment is updated in place on each push.

Review details

No findings block the merge.

FINDING — build.py:3367 — the external-persona branch sets anchor = path.parent from the unresolved absolute path _resolve_prompt_path returns (line 1249 returns path, not the distinct resolved), then passes it to _open_dir_nofollow_pinned(anchor, already_resolved=True) at 3430, which per-component O_NOFOLLOW-walks those unresolved components from /; on any host where an ancestor is a symlink (macOS /tmp→/private/tmp, /var, a symlinked $HOME) the walk raises OSError → ExportRefused, aborting the whole bundle for the supported absolute-persona-outside-agents_dir case — the exact "walking from / with O_NOFOLLOW would refuse any legitimate path whose ancestors include a symlink" the code's own comment at 3346-3350 says this branch must avoid (Linux /tmp isn't a symlink, so CI stays green) → Fix: resolve the outside anchor before pinning — anchor = path.parent.resolve(), keeping already_resolved=True, so the no-follow walk runs over canonical components and reduces to the intended final-component check.

[OPUS-REVIEWED] 8cd7599

Verdict parsed from the review's SHA-scoped output markers for commit 8cd7599ee0aad503b56dff1a793dffd189feeb6e.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 8cd7599ee0aad503b56dff1a793dffd189feeb6e: <one-sentence reason>

[chenmingwei23]

/ai-review override gpt 5b71879: Provider REFUSED (not a finding); gate says re-runs do not clear this class. Diff is one pure-Python package + 17 tests, no AWS/Docker/network, main() uncalled by design, security guards each pinned by a mutation test, 137 pass, Opus did not block.

[chenmingwei23]

Rebuttal: concurrent staging claim, build.py staging.mkdir(parents=True)

OUT OF SCOPE, with the evidence already in this branch rather than an argument.

A translation of FileExistsError was written here and then removed, because a mutation showed it was unreachable: deleting the try/except left all 242 packaging tests passing.

Every way the staging path can already exist is refused above the claim, each with a message that names the cause better than the mkdir could:

• a link or junction at the path
• a path that exists and is not a directory
• a tree without this build's own marker (the staging path ... already exists and this build did not create it)
• a tree holding files this build does not own

The case those checks were thought to miss is an empty directory, on the reasoning that exists() and not is_dir() is False for one and an empty tree holds no unowned files. It is refused too, by the marker check. That is pinned by tests/test_sensitive_source_and_report_identity.py::test_an_empty_staging_directory_is_refused_by_the_marker_check, which asserts the refusal names this build did not create and fails if the marker check ever moves below the claim.

What remains is a race between the marker check and the mkdir in a second process. No test on one process can enter it, so a guard there is an error path nothing can redden. The repository's own rule is that such a guard is removed and the ordering pinned instead, which is what this branch does; the reasoning is recorded at the call site.

exist_ok stays False deliberately: creating the directory is how the build CLAIMS the path, and exist_ok=True would let two builds share one staging tree. That is pinned separately by test_the_claim_is_still_the_mkdir.

[bolichen97]

@chenmingwei23 From the open-PR relationship audit (audited at 0636f23). This PR overlaps #9223 in exactly two files, and the two are complementary rather than competing, so both should land.

Where they overlap. Both PRs add src/kiro_crew/apps/builtins/aws_control/crew/__init__.py as a new file with byte-identical content (same blob index b1e16bed3), so it is an add/add collision for whichever lands second. Both also edit test/test_spawn_audit.py, but in disjoint regions: this PR appends three packaging CLI entries to BENIGN_SPAWNS, while #9223 adds _is_container_image_asset, two walk exclusions, and test_container_image_assets_are_not_imported.

What differs. This PR is the bundle producer (crew/packaging/build.py). #9223 is the image and runtime that consumes the bundle: its runtime declares BUNDLE_ENTRIES as manifest.json, agent.json, mcp.json and skills, its Dockerfile copies those three files, and it re-derives the digest skipping manifest.json, which is exactly this PR's bundle_digest contract. #9223 has no files under crew/packaging/, and nothing under crew/runtime/ imports crew.packaging, so there is no behavioural conflict.

Suggestion. Pick one owner for crew/__init__.py and have the other PR drop it instead of re-adding it. That docstring documents scripts/, templates/ and packaging/, and only packaging/ arrives with this PR, so if #9223 lands first the docstring points at nothing. Keeping the file here is the cleaner split.

Separately, #8470 duplicates all 17 files of this PR's packaging slice, 10 of them byte-identical. It should rebase and take this PR's build.py rather than land its own older copy.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.

[github-actions]

GPT 5.6 Review — ⚠️ review incomplete

GPT 5.6 did not produce a complete verdict for 83eee305652c1af1b7a101fa92b3757159a6dc70; inspect the workflow logs and re-run it.

This comment is updated in place on each push.

See the GPT 5.6 Review job logs; this commit has no completed GPT verdict.

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 83eee305652c1af1b7a101fa92b3757159a6dc70: <one-sentence reason>

[github-actions]

UX Review (Fable 5) — ⏭️ skipped

Revision 8cd7599ee0aad503b56dff1a793dffd189feeb6e touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Advisory — does not block merge.