Skip to content

Close the --force flag-verb bypass #303

Closed

Description

@blooop

Question

dl --force <ws> --rm force-deletes: verb_command reads cli.force with no positional check (rust/dl/src/cli.rs:756) while workspace_command consults force_placement (cli.rs:847-882) and refuses the same misparse for the word verbs. The guard's own rationale ("clap-parsed Rust silently accepted the flag and deleted") applies identically to both spellings.

Route verb_command through the same placement check, red-first: failing grammar tests for dl --force <ws> --rm and dl --rm --force <ws> (must refuse and delete nothing), while dl <ws> --rm --force keeps working.

Activity

  1. self-assigned this
    on Aug 22, 2026
  2. blooop commented on Aug 22, 2026

    @blooop
    OwnerAuthor

    breadcrumb: entering building — wf-tdd stage subagent launched (manager: wf-mid run working map #299).

  3. blooop commented on Aug 22, 2026

    @blooop
    OwnerAuthor

    breadcrumb: building. Restating the contract: verb_command (the --stop/--rm flag-verb grammar) folds --force in wherever clap found it, while the word grammar recovers --force's position and refuses it anywhere but trailing. Fix: route the flag-verb grammar through the same placement check, so both spellings of a verb read --force identically.

    Seams (as the ticket pre-agreed — grammar tests at the resolver): dl --rm --force <ws> refuses the way dl rm --force <ws> already does (verb-slot --force); dl --force <ws> --rm reads --force as the workspace name the way the word grammar does, forcing nothing; dl <ws> --rm --force and dl --rm <ws> --force keep meaning forced remove (trailing slot). Two slices: verb-slot first, workspace-slot second.

  4. blooop commented on Aug 22, 2026

    @blooop
    OwnerAuthor

    breadcrumb: build complete, PR #344 pushed. The builder subagent was killed by a spend limit mid-verification, having reported the suite "flaky under parallel load"; re-run alone by the manager it is clean (1,578 pass, 20 suites, zero failures) — the flakiness was contention from five concurrent agents, not a defect. No code was written outside the builder's commits.

  5. blooop commented on Aug 22, 2026

    @blooop
    OwnerAuthor

    breadcrumb: build → in review. PR #344 green on every check (ci, rust, rust-coverage, e2e, public-api, packaging, prek, gate, codecov). Handing to a fresh wf-review context.

  6. blooop commented on Aug 22, 2026

    @blooop
    OwnerAuthor

    breadcrumb: review complete — Request changes. Standards ✅, Spec ✅ (both axes pass on their own terms), but the stage's own safety probe of the placement space found two blockers: a non-trailing --force still force-deletes on flag-verb lines (dl --rm stop --force ws → Remove { force: true }, because the verb flag consumes a slot so index 2 is mid-line), and dl --rm --force (selector + force) regressed with no test while the retired-word spelling dl --rm prune --force still works. Full report on #344.

  7. blooop commented on Aug 22, 2026

    @blooop
    OwnerAuthor

    breadcrumb: in review → changes requested, routed back to building. The reviewer's own probe of the full placement space (not the diff's named cases) found two blockers, and one of them needs a call.

    agent-decided (simplicity): dl --rm --force — the selector form with no positional — must keep working. It worked at merge-base and the diff now refuses it, with no test and no recorded decision.

    The guard exists because --force sitting in a name slot is ambiguous: it may be the flag, or it may be the workspace the user meant to delete. A selector line carries no positional at all, so that ambiguity cannot arise — there is nothing for --force to be mistaken for. A refusal there guards nothing and costs a working invocation, which makes it a regression the ticket never asked for. Cheap to reverse either way, so this is not worth a human question.

    What nearly won: "refuse every --force that is not trailing, uniformly", which is easier to state and easier to keep true. It loses because the uniformity is only apparent — the retired spelling dl --rm prune --force still resolves to a forced remove today, so the rule would not actually hold, and buying a simpler sentence with a broken working command is the wrong trade under principle 1.

  8. blooop commented on Aug 22, 2026

    @blooop
    OwnerAuthor

    breadcrumb: changes requested → building, both blockers closed red-first on PR #344 (two commits, no force-push).

    Both turned out to be one defect. The placement guard counted the verb flag as a name slot, and a flag verb is not a name — it is the verb. That shift showed at both ends: a mid-line --force read as trailing and force-deleted, and a --force right after the flag refused with the flag itself named as the workspace. The rule is now asked of the stream the flag is not in, and stated as "no name follows it" rather than "past slot two" — the second half matters, because with the flag out the longest line left is a name, a name and a --force, so asking for slot two exactly would be the same off-by-one from the other end. Both spellings of a verb now give the same answer to the same positional shape, which is what the ticket asked for.

    That phrasing is also what delivers the manager's call on the selector form: a line naming no workspace has nothing for --force to be mistaken for, so it keeps meaning a forced remove — earned by the rule rather than special-cased beside it. Pinned in both token orders and in the retired spelling that says the same thing, so a future tightening cannot take it back by accident.

    The reproduction is worth keeping as a habit: a matrix over every ordering of the slot-carrying tokens, holding one property — a forced delete implies nothing named follows --force — rather than another handful of named cases. Named cases are what missed this, and the matrix's first red was a three-token line no report had named, so the defect was never only in the long shapes. The destructive end is pinned at the binary boundary too, against a scratch cache, where the failing run really had removed the clone.

    Folded in from the non-blocking list: the doc-comment that contradicted its own arm, the duplicated verb-slot refusal, the repeated flag literal. Left alone deliberately: aid's leading --rm --force (another crate) and the grammar/README doc line, which needs both files edited together or it becomes the drift CLAUDE.md forbids — worth a follow-up.

  9. blooop commented on Aug 22, 2026

    @blooop
    OwnerAuthor

    breadcrumb: changes requested → back in review. Head df385b1, all 12 checks green.

    Worth recording: the two blockers were one defect. The placement guard counted the verb flag as a name slot, and a flag verb is not a name — it is the verb. That off-by-one showed at both ends: a mid-line --force read as trailing (B1), and a --force right after the flag refused with the flag itself named as the workspace (B2). The rule is now asked of the stream the verb flag is not in and stated as "no name follows it" rather than "past slot two" — so dl --rm --force falls out of the phrasing rather than being special-cased, which is a better outcome than the manager decision above asked for.

    The matrix test earned its place immediately: its first red was dl ws --rm --force other → a forced remove, a three-token workspace-first line neither the review nor my decision had named. The named cases were never the whole defect — which is the second time on this map that a matrix over the input space found what enumerated cases missed.

  10. blooop commented on Aug 22, 2026

    @blooop
    OwnerAuthor

    breadcrumb: re-review at df385b1 — both blocking findings resolved, verdict Approve. B1: flag_verb_force_placement filters the verb flag out of the slot stream and honours --force only when no name follows it — verified equivalent to the invariant at both ends (the filtered stream is provably just the words plus --force), and every shape from the report now refuses with the word spelling's own sentence. B2: dl --rm --force is Select { Remove { force: true } } again, pinned. The reported first red dl ws --rm --force other is real and was a shape nobody had named. Re-ran the builder's property myself over a wider alphabet (rm/up/code added) to five tokens: 5370 lines, 620 forced deletes, no counterexample; sensitivity confirmed by porting the same probe onto 12a5d8f, where it fails. Five non-blocking follow-ups noted on the PR, the notable one being that the word grammar still uses the absolute-index phrasing and the matrix alphabet omits rm, so that half is unguarded — latent, not live. Full report on #344.

  11. blooop commented on Aug 22, 2026

    @blooop
    OwnerAuthor

    breadcrumb: in review → approved, awaiting human merge. Both blocking findings resolved at head df385b1; overall verdict Approve.

    The reviewer did not just re-check its findings — it established the new rule's equivalence by construction, enumerating why the filtered slot stream is provably exactly the positional words plus --force (--yes/--autorm refused earlier, --devcontainer+value stripped, a second verb flag a clap group conflict, --json/--size gated, repeats an ArgumentConflict, past -- out of stream), so "last in stream" ⟺ "no name follows in argv" at both ends. It then re-ran the property over a wider alphabet to five tokens — 5,370 resolving lines, 620 forced deletes, no counterexample — and proved its own probe sensitive by porting it onto 12a5d8f, where it fails on dl ws other --rm --force rm.

    Its most useful non-blocking finding is graduated to #353: the word grammar's half of the rule is unguarded in the same way, because workspace_command still uses absolute-index phrasing that only works while clap caps positionals at 2, and the new matrix alphabet omits rm — the one token that makes the word grammar produce a forced delete. Latent, not live; the reviewer confirmed it passes with rm added.

    This ticket stays open until PR #344 is merged.

  12. blooop commented on Aug 24, 2026

    @blooop
    OwnerAuthor

    This was generated by AI during a retrospective review.

    Closing: the bypass is dead, but not because #344 landed — #344 was closed unmerged. #369 killed it as a side effect of the --rm rework.

    verb_command, the function this issue names, was deleted entirely. --rm is no longer a delete verb, and cli.rs:772 now refuses the pair unconditionally, ahead of force_placement and ahead of every other check:

    if cli.rm && cli.force {
        return Err(GrammarError::RmForced);
    }

    Because it runs before position is consulted, no ordering escapes it. Verified by re-running #344's own argv-space property as a temporary matrix — length ≤4 over 14 tokens, length ≤5 over 6 — against four invariants:

    • a forced delete implies nothing named follows --force
    • --rm + --force always yields RmForced
    • a retired spelling always refuses
    • --rm never deletes immediately, rm never defers

    No escapes. Including dl ws --rm --force other, the three-token line #344's matrix found and no hand-written case had named. The matrix bites rather than passing vacuously: disabling force_placement produces 66 escapes, removing the RmForced guard produces 284.

    #354's sub-item "aid's leading --rm --force loses force" is dead for the same reason.

    What did not come across

    A guard. The property lives only as five hand-named orderings at cli.rs:1645-1662; #344's matrix never landed. That residue is #354, which should stay open — the behaviour is correct today and nothing would notice if a future edit to the grammar broke it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions