fix(cli): resolve run/create's bare NAME_OR_DIR instead of mkdir'ing it - #272
Merged
Conversation
With an `izba.yml` in the cwd declaring `metadata.name: my-sandbox`,
`izba run -d my-sandbox` created an empty `./my-sandbox/`, found no manifest
there, and booted a default-image sandbox with egress enforcement OFF — while
`izba run -d --name my-sandbox` honoured the manifest. Two spellings of the
same intent produced different sandboxes, and the divergence landed on the
security posture with no warning at either site: the only evidence a declared
`enforce:`/`protocol:` posture had been discarded was a stray directory the
user never asked for.
`resolve_or_create` tested only whether the positional named an EXISTING
sandbox; anything else fell straight through to `ensure_workspace`'s
`create_dir_all`. A mistyped name therefore became a new workspace inside the
user's repository rather than an error.
Add `sandbox_ref::resolve_for_create`, the create-capable counterpart of
`resolve`, and route both `run` and `create` through it:
1. path syntax (`.`, `./proj`, any separator) -> that workspace directory —
the ONE form that may be created if missing;
2. bare word naming an existing sandbox -> that sandbox;
3. bare word with a `./word/izba.yml` -> that workspace (with a note),
mirroring `resolve`;
4. bare word equal to the cwd manifest's `metadata.name` -> the project
sandbox that `--name <word>` and `.` already reach;
5. 3 and 4 both matching -> a hard error naming both interpretations;
6. anything else -> an actionable error, having created NO directory.
Arm 4 is not a new trust surface for the agent-writable `izba.yml`: it is
taken only when the manifest's name EQUALS the name the user typed, so a
manifest can confirm the target but never redirect it to a different sandbox.
`resolve` itself is UNTOUCHED — #159 stays deliverable against it as it
stands, and the two resolvers share `is_path_syntax` by construction rather
than by convention, so the syntactic split cannot drift.
`ensure_workspace` keeps its `create_dir_all`, but is now reachable only
through the resolver, which is what confines implicit directory creation to
path syntax. A new caller that bypasses the resolver reopens exactly that
hole, so the invariant is stated at the function.
Also warn whenever a cwd `izba.yml` is not the manifest being applied, so a
discarded declaration is never silent. The warning keys on the applied
WORKSPACE, not the sandbox name: `izba create --name other .` does apply the
cwd manifest, and a false alarm there would train users to ignore the one
message that reports a dropped `enforce:`. An UNPARSEABLE cwd manifest warns
too — "I could not read it" is precisely when a dropped `enforce:` would
otherwise go unmentioned.
Resolution now runs before `DaemonClient::connect`, so a bare-word miss no
longer pays for spawning izbad — pinned by a test, alongside the existing
`ensure_socket_budget` early-exit for #71.
`create`'s positional becomes NAME_OR_DIR (a `String`, matching `run`) and
both verbs' help text describes the rule actually implemented. README's
"Referring to sandboxes" — the canonical statement of the rule — now lists
`create` and `run` alongside the seven verbs that already shared it.
Drive-by, found while auditing bare-word call sites: `sandbox.rs`'s
image-cache-repair remediation told users to run
`izba create izba-cache-repair --image ...`, a bare word this rule rejects.
No test pinned that string, so it would have shipped as silent doc-rot; it
now spells the target `--name izba-cache-repair .`.
`create_sunlen_failures.rs` spells its target `--name web .` for the same
reason: a bare `web` is now rejected by the resolver, which sits ahead of the
SUN_LEN check in `create` and would have pre-empted the error that file
exists to pin.
Verified: all six workspace gates; the app backend gate; the full 13-test
KVM `daemon_e2e` suite on real microVMs, including a new step [12] that
proves the live acceptance criterion — the bare-name and `--name` spellings
reach the same sandbox and produce byte-identical `izba policy show` output
with the manifest's `enforce: true` in effect. cargo-mutants over the new
code: 14 caught, 1 unviable, 0 survivors.
Refs #242
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s decision The incremental mutation gate found one survivor: `replace warn_ignored_cwd_manifest with ()` in run.rs. The wrapper was a thin printer around a pure, already-unit-tested decision, and BOTH of its call sites sit after `DaemonClient::connect` inside `resolve_or_create` — which is `#[mutants::skip]`ed as live-daemon/e2e-only. So no host test could ever reach the wrapper, and factoring it out bought nothing but an unkillable mutant. Inline it at both call sites instead, inside the already-skipped function where it honestly belongs, and state at the site why it is not a helper. That alone would only move the problem, so also cover the call site for real. `create` emits the same warning BEFORE its daemon connect, so it IS reachable without a daemon: two binary-level tests now pin that `izba create ./sub` from a directory holding an unrelated `izba.yml` names the manifest it ignored, and that `izba create .` — which does apply the cwd manifest — stays quiet. Both were watched to fail with the call site removed. This is the defect class this repo keeps rediscovering: a RULE with a test and a CALL SITE without one. The rule had thirteen unit tests; the call site had none, and a mutant is what noticed. Refs #242 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Greptile (3/5) caught a real gap. `resolve` refuses a bare word that is BOTH
an existing sandbox AND a directory whose `izba.yml` resolves to a DIFFERENT
sandbox — two live meanings, so it asks for `./word` or the exact name.
`resolve_for_create` was written without that rail, so the two resolvers
disagreed on the same argument: `izba status myapp` refused it while
`izba run myapp` attached to sandbox `myapp` and `./myapp/izba.yml` was
neither applied nor mentioned.
That is the wrong-target-with-silently-discarded-posture class this whole
change exists to close, reintroduced one arm to the left of where it was
fixed. I had left the rail out deliberately to keep the diff small; that was
the wrong call, and a test now proves it: before this commit
`resolve_for_create(paths, "proj")` returned `Existing("proj")` where it had
to refuse.
Extract the rail into `reject_ambiguous_existing` and call it from BOTH
resolvers rather than copying it into the second. Duplicating it is exactly
how they drifted the first time, so a test asserts the two produce the
IDENTICAL refusal string for the same input — it fails if either side is
changed alone. `resolve`'s behaviour is unchanged (a pure extraction, pinned
by its existing `ambiguous_bare_word_is_a_hard_error` and
`agreeing_bare_word_resolves_as_the_sandbox`).
Also make the tests' `with_cwd` helper restore the cwd from a Drop guard
instead of a trailing statement. A failing assertion inside it panics, and
with a trailing restore the process cwd stayed inside a tempdir that was then
deleted — plus the panic poisoned `CWD_LOCK`, so every later cwd-dependent
test failed too. One real assertion failure read as twenty, which is exactly
how a genuine regression gets dismissed as flakiness.
Verified: all six workspace gates; the full 13-test KVM `daemon_e2e` suite on
real microVMs, still green; cargo-mutants over the new and changed code —
17 caught, 1 unviable, 0 survivors.
Refs #242
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Lupus
force-pushed
the
fix/run-bare-name-manifest
branch
from
September 1, 2026 19:46
00df285 to
fa9ab3c
Compare
|
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.



Closes #242
The bug
With an
izba.ymlin the cwd declaringmetadata.name: my-sandbox:izba run -d --name my-sandboxenforce: truein effectizba run -d my-sandbox./my-sandbox/, found no manifest there, booted a default-image sandbox withenforce: offTwo spellings of the same intent produced different sandboxes, and the divergence landed on the security posture with no warning at either site. The only evidence that a declared
enforce:/protocol:posture had been discarded was a stray directory the user never asked for.The cause:
resolve_or_createtested only whether the positional named an existing sandbox; anything else fell through toensure_workspace'screate_dir_all. So a mistyped name silently became a new workspace inside the user's repository instead of an error.The rule
The issue deliberately left two design forks open. Both were settled with the repo owner before implementation:
metadata.nameresolves to the project sandbox (rather than being refused with a pointer to--name/.).create_dir_allsurvives only for path syntax (rather than being kept with a loud note, or removed entirely).New
sandbox_ref::resolve_for_create— the create-capable counterpart ofresolve— now ownsrun/create's positional:.,./proj, any separator) → that workspace directory — the one form that may be created if missing;./word/izba.yml→ that workspace (with a note), mirroringresolve;metadata.name→ the project sandbox--name <word>and.already reach;Why arm 4 is not a new trust surface
izba.ymlis agent-writable and therefore an untrusted proposal. Arm 4 is taken only when the manifest's name equals the name the user typed, so a manifest can confirm the target but never redirect it to a different sandbox. It also trusts no content the other two spellings from the same cwd did not already trust.Acceptance criteria
izba policy showreports the manifest'senforce:/protocol:for both — proven live in newdaemon_e2estep [12], which asserts the two invocations yield byte-identicalpolicy showoutput./my-sandbox/left behind — asserted in the same e2e step and in unit + binary-level testsizba.ymlthat is not the manifest applied produces a warning naming itNAME_OR_DIRhelp text matches the implemented rule (create's positional is nowNAME_OR_DIR: String, matchingrun)., and--nameequivalencepolicyhelp falsely promises "(or dir)" #159 covers —resolve's behaviour is unchanged (its ambiguity rail was extracted into a sharedreject_ambiguous_existing, a pure refactor pinned by its existing tests);resolve_for_createis otherwise purely additiveDesign notes for review
izba create --name other .genuinely does apply the cwd manifest, and a false alarm there would train users to ignore the one message that reports a droppedenforce:. An unparseable cwd manifest warns too — "I could not read it" is precisely when a discardedenforce:would otherwise go unmentioned.is_path_syntaxANDreject_ambiguous_existingby construction, not by convention, so neither the syntactic split nor the ambiguity rail can drift between them. A test asserts both produce the identical refusal string for the same input, so it fails if either side is changed alone.izba.ymlnames a different sandbox is refused, exactly asresolvealready refused it.ensure_workspacekeeps itscreate_dir_allbut is now reachable only through the resolver, which is what confines implicit creation to path syntax. A new caller bypassing the resolver reopens exactly that hole, so the invariant is stated at the function rather than left implicit.DaemonClient::connect— a bare-word miss no longer pays for spawningizbad. Pinned by a test, alongside the existingensure_socket_budgetearly-exit for daemon/egress:izba runfails ('path must be shorter than SUN_LEN') when IZBA_DATA_DIR is deep — runtime socket path exceeds 108-char unix limit #71.izba run my-sandboxresolves via the cwd manifest pre-create, whileizba status my-sandboxstill errors until the sandbox exists. That divergence exists only before creation — exactly the window whererun/createare the relevant verbs — and keeps Sandbox cwd/dir resolution is half-applied across the CLI;policyhelp falsely promises "(or dir)" #159 deliverable againstresolveas it stands.Drive-by fixes found while auditing bare-word call sites
crates/izba-core/src/sandbox.rs— the image-cache-repair remediation instructed users to runizba create izba-cache-repair --image ..., a bare word this rule rejects. No test pinned that string, so it would have shipped as silent doc-rot. Now spells the target--name izba-cache-repair ..create_sunlen_failures.rs— both cases now spell the target--name web .. A barewebis rejected by the resolver, which sits ahead of the SUN_LEN check increateand would have pre-empted the error that file exists to pin. (Itsrunsibling passed only by an ordering coincidence; it is no longer order-dependent.)README.md"Referring to sandboxes" — the canonical statement of the rule — now listscreateandrunalongside the seven verbs that already shared it.Verification
cargo test --workspacecargo clippy --workspace --all-targets -- -D warningscargo fmt --checkizba-initmusl static buildcargo check/clippy -D warningsapp/src-taurifmt + clippy + testdaemon_e2e, all 13 tests on real microVMscargo-mutantsover the new resolver codeDeveloped test-first: every new function had a failing test watched to fail before implementation.
Changes made during review
Greptile (3/5) found a real gap, fixed in
00df285c.resolverefuses a bare word that is both an existing sandbox and a directory whoseizba.ymlresolves to a different sandbox.resolve_for_createwas written without that rail, so the two resolvers disagreed on the same argument:izba status myapprefused it whileizba run myappattached to sandboxmyappwith./myapp/izba.ymlneither applied nor mentioned — the wrong-target-with-silently-discarded-posture class this PR exists to close, reintroduced one arm to the left. I had omitted the rail deliberately to keep the diff small; that was the wrong call. A test now proves it: before the fix,resolve_for_create(paths, "proj")returnedExisting("proj")where it had to refuse.The rail is now extracted and shared rather than copied — duplication is how the two drifted in the first place.
The mutation gate found a second real gap, fixed in
33bc82d6.warn_ignored_cwd_manifestwas a thin printer whose two call sites both sit afterDaemonClient::connect, so no host test could ever reach it — the helper bought nothing but an unkillable mutant. Inlined into the already-#[mutants::skip]ed function, and the call site covered for real viacreate, which emits the same warning before its connect.Test-harness fix:
with_cwdnow restores the cwd from a Drop guard. A failing assertion inside it panics; with a trailing restore the process cwd stayed in a deleted tempdir and the panic poisonedCWD_LOCK, so one genuine failure read as twenty — exactly how a real regression gets waved off as flakiness.Re-verified after both fixes: all six workspace gates, the full 13-test KVM
daemon_e2esuite on real microVMs, and cargo-mutants at 17 caught / 1 unviable / 0 survivors.🤖 Generated with Claude Code
Greptile Summary
The PR changes
createandrunso bare names resolve without implicitly creating directories, while explicit path syntax remains create-capable.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[create or run NAME_OR_DIR] --> B{Path syntax?} B -- Yes --> C[Resolve workspace path] C --> D[Create directory if missing] B -- No --> E{Existing sandbox?} E -- Yes --> F{Conflicting subdirectory manifest?} F -- Yes --> G[Reject as ambiguous] F -- No --> H[Use existing sandbox] E -- No --> I{Subdirectory or matching cwd manifest?} I -- Both --> G I -- Subdirectory --> C I -- Cwd manifest --> J[Use current workspace] I -- Neither --> K[Reject without filesystem mutation]Reviews (4): Last reviewed commit: "fix(cli): share the ambiguous-bare-word ..." | Re-trigger Greptile
Context used: