Skip to content

fix(stibbons): harden labels sync arg handling and subprocess safety#721

Merged
joshjhall merged 1 commit into
mainfrom
feature/issue-694
Jul 5, 2026
Merged

fix(stibbons): harden labels sync arg handling and subprocess safety#721
joshjhall merged 1 commit into
mainfrom
feature/issue-694

Conversation

@joshjhall

Copy link
Copy Markdown
Owner

Summary

Combined hardening of stibbons labels sync, addressing three deferred
follow-ups from the adversarial review of #289 / PR #693. All changes are
confined to crates/stibbons/src/labels/, so they ship as one PR.

#694 — arg-injection hardening

  • LabelDef::validate() rejects name/color/description values beginning
    with - (which gh/glab could otherwise parse as a flag) and enforces a
    6-hex-digit color.
  • Backends build argv with =-form flags (--color=…, --description=…) and,
    for gh, a -- terminator before the positional name — so a leading-dash
    value can never be parsed as an option even if validation were bypassed.

#695 — subprocess timeouts + walk/size guards

  • New exec::run_with_timeout wraps gh/glab/git in a std-only deadline
    (spawn + thread-drained pipes + try_wait poll + kill), so a wedged
    interactive re-auth or a stalled network call fails the run instead of hanging
    forever (fatal in CI). backend::run() and platform::origin_remote_url()
    route through it. No new dependencyCargo.lock is untouched.
  • find_metadata_files tracks canonicalized visited directories and caps
    recursion depth, terminating on directory symlink cycles instead of exhausting
    fds/stack. Oversized metadata.yml files are skipped with a warning before
    parsing.

#696 — test coverage

  • gh/glab create/update arg construction asserted (incl. the GitLab
    --label-id update path and the gh -- terminator); validate()
    accept/reject cases.
  • classify_remote bare-substring fallback branch; resolve_skill_roots
    empty-roots error + default_skill_roots() existence; exec timeout/success/
    spawn-failure/large-output.
  • Hermetic integration tests for stibbons setup real execution and the
    orphaned-reference warning reaching CLI stderr.

No behavior change for well-formed label metadata.

Test plan

  • cargo test -p stibbons — 164 unit + 28 labels integration (+ others) pass.
  • cargo clippy --workspace --all-targets -- -D warnings clean; cargo fmt clean.
  • Manual: a metadata.yml with a leading-dash color now errors clearly at
    validate() before any mutating gh label edit call (exit 1), instead of
    shelling a bad flag.

Closes #694
Closes #695
Closes #696

Deferred hardening follow-ups from the adversarial review of #289 / PR #693,
all confined to crates/stibbons/src/labels/ and shipped together.

Arg-injection hardening (#694):
- LabelDef::validate() rejects name/color/description beginning with `-`
  (which gh/glab could parse as a flag) and enforces a 6-hex-digit color.
- Backends build argv with `=`-form flags (--color=..., --description=...)
  and, for gh, a `--` terminator before the positional name, so a
  leading-dash value can never be parsed as an option even if validation is
  bypassed.

Reliability guards (#695):
- New exec::run_with_timeout wraps gh/glab/git in a std-only deadline
  (spawn + thread-drained pipes + try_wait poll + kill), so a wedged
  interactive re-auth or stalled network fails the run instead of hanging.
  backend::run() and platform::origin_remote_url() route through it.
- find_metadata_files tracks canonicalized visited dirs and caps recursion
  depth, terminating on directory symlink cycles instead of exhausting
  fds/stack; oversized metadata.yml is skipped with a warning before parsing.

Test coverage (#696):
- gh/glab create/update arg construction asserted (incl. GitLab --label-id
  update path and the gh `--` terminator); validate() accept/reject cases.
- classify_remote bare-substring fallback branch; resolve_skill_roots
  empty-roots error and default_skill_roots existence; exec timeout/success.
- Hermetic integration tests for `stibbons setup` real execution and the
  orphaned-reference warning reaching CLI stderr.

No new dependency (Cargo.lock untouched); no behavior change for well-formed
metadata.

Closes #694
Closes #695
Closes #696

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@joshjhall joshjhall merged commit b922712 into main Jul 5, 2026
16 checks passed
@joshjhall joshjhall deleted the feature/issue-694 branch July 5, 2026 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants