jbr870/devwork-skills
1
0
Fork 0
Code Issues 64 Pull requests Projects Releases Packages Wiki Activity Actions

dw.sh hardening: concurrency, locking and reattach edge cases (15 findings from #61 QA round 1) #353

New issue
Open
opened 2026-08-26 14:36:12 +00:00 by jbr870 · 17 comments
jbr870 commented 2026-08-26 14:36:12 +00:00
Owner
Copy link

Spawned from finding F-PO-61-4-3 (in-scope-deferrable) on issue #61 during decision D-PO-61-4-2,
QA round 1. Fifteen findings, all raised by the round-1 code domain validate (issue #61, comment 1924,
finding ids CR-19..CR-32 and CR-34), batched here because they share one theme: concurrency,
locking and reattach edge cases in .devcontainer/dw.sh.

Two of the fifteen are NOT hardening and should be triaged first.

  • CR-23 — require_issue accepts leading zeros. dw.sh up 07 and dw.sh up 7 build rival
    containers (dw-07 and dw-7) against the same logical issue, and eventually two branches for it.
    This is a correctness bug, not an edge case, and it justifies its own fix rather than a slot in a
    hardening sweep.
  • CR-24 / CR-30 — peer-churn false failures. AC-4 and AC-11 assert the host credential file is
    unchanged across multi-minute scenarios; AC-3 reads the integration tip before up. On this box,
    parallel sessions are the working model, so a legitimate host-side OAuth refresh or a peer merging to
    main mid-scenario reports as a violation. These cost debugging time rather than correctness, and
    CLAUDE.md already records one full sweep invalidated by peer churn.

The rest, grouped:

Locking and concurrency — CR-21 cmd_rm takes no per-issue lock, so rm can interleave with a
concurrent up (TOCTOU between inspection and rm -f); CR-22 acquire_lock's stale-lock clearing
lets two waiters both observe the dead pid, so the slower one deletes the lock the faster just
acquired; CR-28 resolve_branch's host-side git ls-remote has no timeout and runs while the
per-issue lock is held, so a blackholed forge blocks that up indefinitely and any concurrent up for
the full 600s.

Reattach and state — CR-19 a partial clone (SIGKILL or host reboot mid-clone) leaves /workspace/.git
present, setup_workspace early-returns on -d .git, and the container is marked ready on a corrupt
workspace; CR-25 --ssh-agent is silently ignored whenever the container already exists, with no
warning that recreate is the fix; CR-31 the is_ready early return skips verify_forge_ssh, so a
rotated forge host key surfaces mid-session at push time instead of at start.

Failure modes that degrade quietly — CR-29 a malformed host settings.json degrades to {} with no
warning, so the container silently loses the operator's permission mode (which an unattended run needs);
CR-26 the attach path execs, so the EXIT trap never runs and $TMPROOT leaks on every interactive
up; CR-20 the forge token is passed as a plaintext argv to curl and tea login add, visible in the
process table.

Test-shape — CR-27 setup_forge_login's delete-then-add ordering (see also RV-4 on #61, which is
being fixed there); CR-32 AC-16 verifies "setup did not re-run" by grepping for hardcoded message text,
so a reworded message false-FAILs and an always-printed one false-PASSes; CR-34 the round's rebase
auto-resolution, recorded per rebase-onto-base.md.

Full text and per-finding failure scenarios are in the qa-report:v1 at issue #61 comment 1924
(domain=code phase=validate).

This issue was deferred out of the parent feature's scope; it carries no PREQ yet. Run
/dev:requirements --issue={this} to flesh it out before planning.

Spawned from finding `F-PO-61-4-3` (in-scope-deferrable) on issue #61 during decision `D-PO-61-4-2`, QA round 1. Fifteen findings, all raised by the round-1 `code` domain validate (issue #61, comment 1924, finding ids `CR-19`..`CR-32` and `CR-34`), batched here because they share one theme: concurrency, locking and reattach edge cases in `.devcontainer/dw.sh`. **Two of the fifteen are NOT hardening and should be triaged first.** - **`CR-23` — `require_issue` accepts leading zeros.** `dw.sh up 07` and `dw.sh up 7` build *rival* containers (`dw-07` and `dw-7`) against the same logical issue, and eventually two branches for it. This is a correctness bug, not an edge case, and it justifies its own fix rather than a slot in a hardening sweep. - **`CR-24` / `CR-30` — peer-churn false failures.** AC-4 and AC-11 assert the host credential file is unchanged across multi-minute scenarios; AC-3 reads the integration tip before `up`. On this box, parallel sessions are the working model, so a legitimate host-side OAuth refresh or a peer merging to `main` mid-scenario reports as a violation. These cost debugging time rather than correctness, and CLAUDE.md already records one full sweep invalidated by peer churn. **The rest, grouped:** *Locking and concurrency* — `CR-21` `cmd_rm` takes no per-issue lock, so `rm` can interleave with a concurrent `up` (TOCTOU between inspection and `rm -f`); `CR-22` `acquire_lock`'s stale-lock clearing lets two waiters both observe the dead pid, so the slower one deletes the lock the faster just acquired; `CR-28` `resolve_branch`'s host-side `git ls-remote` has no timeout and runs while the per-issue lock is held, so a blackholed forge blocks that `up` indefinitely and any concurrent `up` for the full 600s. *Reattach and state* — `CR-19` a partial clone (SIGKILL or host reboot mid-clone) leaves `/workspace/.git` present, `setup_workspace` early-returns on `-d .git`, and the container is marked ready on a corrupt workspace; `CR-25` `--ssh-agent` is silently ignored whenever the container already exists, with no warning that `recreate` is the fix; `CR-31` the `is_ready` early return skips `verify_forge_ssh`, so a rotated forge host key surfaces mid-session at push time instead of at start. *Failure modes that degrade quietly* — `CR-29` a malformed host `settings.json` degrades to `{}` with no warning, so the container silently loses the operator's permission mode (which an unattended run needs); `CR-26` the attach path `exec`s, so the EXIT trap never runs and `$TMPROOT` leaks on every interactive `up`; `CR-20` the forge token is passed as a plaintext argv to `curl` and `tea login add`, visible in the process table. *Test-shape* — `CR-27` `setup_forge_login`'s delete-then-add ordering (see also RV-4 on #61, which is being fixed there); `CR-32` AC-16 verifies "setup did not re-run" by grepping for hardcoded message text, so a reworded message false-FAILs and an always-printed one false-PASSes; `CR-34` the round's rebase auto-resolution, recorded per `rebase-onto-base.md`. Full text and per-finding failure scenarios are in the `qa-report:v1` at issue #61 comment 1924 (`domain=code phase=validate`). This issue was deferred out of the parent feature's scope; it carries no PREQ yet. Run `/dev:requirements --issue={this}` to flesh it out before planning.
jbr870 commented 2026-08-31 14:05:39 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-74","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"--not --remotes trusts a remote-tracking REF, not the remote itself","original_scope_note":"dw.sh:983. Remote-tracking refs are local files; nothing checks the remote still holds the commit or exists at all. Mechanism reproduced (origin set to /nonexistent.git with a hand-written refs/remotes/origin/main suppresses the unpushed finding), but no realistic route to a stale-but-populated remote-tracking ref exists in dw.sh own workflow — container-init.sh:274 clones from the host real forge origin. The safe direction holds where it matters: a repo with NO remotes reports every commit as unpushed. Becomes real only if a branch is force-deleted or rewritten on the forge after a push and before rm. Deferrable: mechanism real, reachable path unproven.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-74 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-74","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"--not --remotes trusts a remote-tracking REF, not the remote itself","original_scope_note":"dw.sh:983. Remote-tracking refs are local files; nothing checks the remote still holds the commit or exists at all. Mechanism reproduced (origin set to /nonexistent.git with a hand-written refs/remotes/origin/main suppresses the unpushed finding), but no realistic route to a stale-but-populated remote-tracking ref exists in dw.sh own workflow — container-init.sh:274 clones from the host real forge origin. The safe direction holds where it matters: a repo with NO remotes reports every commit as unpushed. Becomes real only if a branch is force-deleted or rewritten on the forge after a push and before rm. Deferrable: mechanism real, reachable path unproven.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:05:42 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-75","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"The selftest `unreadable` shape covers only the top level, which is why 38/38 coexists with CR-69","original_scope_note":"selftest.sh:2623-2626, 2649-2653, 2728. The shape chmod-000s .git/objects AND chmod-700s /workspace. It is NOT vacuous (verified: removing the top-level readability check turns it red and the volume is destroyed), but there is no shape for an unreadable SUBdirectory and none for a submodule. Rolls up into the fix for CR-69/CR-70 — the criterion additions are the same work item, listed separately so the coverage gap is on the record in its own right.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-75 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-75","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"The selftest `unreadable` shape covers only the top level, which is why 38/38 coexists with CR-69","original_scope_note":"selftest.sh:2623-2626, 2649-2653, 2728. The shape chmod-000s .git/objects AND chmod-700s /workspace. It is NOT vacuous (verified: removing the top-level readability check turns it red and the volume is destroyed), but there is no shape for an unreadable SUBdirectory and none for a submodule. Rolls up into the fix for CR-69/CR-70 — the criterion additions are the same work item, listed separately so the coverage gap is on the record in its own right.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:05:46 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-76","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"A commit reachable only from the reflog produces no finding (reasoned, not reproduced)","original_scope_note":"--all walks refs, so a commit orphaned by `git reset --hard` is invisible to the certification. Arguably work the operator deliberately discarded, which is why it is not blocking. Honestly labelled: the reviewer did not build this case.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-76 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-76","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"A commit reachable only from the reflog produces no finding (reasoned, not reproduced)","original_scope_note":"--all walks refs, so a commit orphaned by `git reset --hard` is invisible to the certification. Arguably work the operator deliberately discarded, which is why it is not blocking. Honestly labelled: the reviewer did not build this case.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:05:49 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-77","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"A SIGTERM during the login swap deletes the snapshot before the restore reads it, leaving the container with no forge login","original_scope_note":"container-init.sh:183 (trap on EXIT INT TERM) versus the restore at :193-200. On SIGTERM bash kills the child, runs the handler (deleting the snapshot), and only then enters the failure branch that reads it. Reproduced with a stubbed tea under setsid: `cp: cannot stat .../.dw-tea-bak.Sx7bKo` then the loud `this container has no working forge login now`. It falsifies the invariant stated at :150-152. Deferrable rather than blocking: it is reported loudly with the right remedy, the token still exists on the host, nothing is destroyed that exists nowhere else, and the shipped path is `docker exec` without -t, where a host Ctrl-C is not forwarded and `docker stop` signals PID 1 rather than the process it started. Fix shape: restore-then-remove in the handler, or trap EXIT only.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-77 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-77","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"A SIGTERM during the login swap deletes the snapshot before the restore reads it, leaving the container with no forge login","original_scope_note":"container-init.sh:183 (trap on EXIT INT TERM) versus the restore at :193-200. On SIGTERM bash kills the child, runs the handler (deleting the snapshot), and only then enters the failure branch that reads it. Reproduced with a stubbed tea under setsid: `cp: cannot stat .../.dw-tea-bak.Sx7bKo` then the loud `this container has no working forge login now`. It falsifies the invariant stated at :150-152. Deferrable rather than blocking: it is reported loudly with the right remedy, the token still exists on the host, nothing is destroyed that exists nowhere else, and the shipped path is `docker exec` without -t, where a host Ctrl-C is not forwarded and `docker stop` signals PID 1 rather than the process it started. Fix shape: restore-then-remove in the handler, or trap EXIT only.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:05:53 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-78","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"CR-38 can no longer detect the defect its own comment describes, because round 4 flock removed the overlap it builds","original_scope_note":"selftest.sh:2420-2500. Instrumented timestamps show run B whole life is the 30ms after A releases the lock. Four-way mutation: pristine PASS; shared backup path with the lock kept PASS (the defect the comment names is masked); lock removed with per-run backup FAIL; both removed FAIL. Ranked deferrable rather than passes-on-broken-code because under the lock a shared backup path is genuinely safe — the mutant is not broken code. What is stale is the comment. Note the good news in the same experiment: CR-38 part 2 DOES catch removal of the flock, which matters because the string flock appears nowhere in selftest.sh.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-78 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-78","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"CR-38 can no longer detect the defect its own comment describes, because round 4 flock removed the overlap it builds","original_scope_note":"selftest.sh:2420-2500. Instrumented timestamps show run B whole life is the 30ms after A releases the lock. Four-way mutation: pristine PASS; shared backup path with the lock kept PASS (the defect the comment names is masked); lock removed with per-run backup FAIL; both removed FAIL. Ranked deferrable rather than passes-on-broken-code because under the lock a shared backup path is genuinely safe — the mutant is not broken code. What is stale is the comment. Note the good news in the same experiment: CR-38 part 2 DOES catch removal of the flock, which matters because the string flock appears nowhere in selftest.sh.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:05:56 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-79","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"No criterion covers a signal arriving during the login swap","original_scope_note":"Signal assertions in the harness cover container stop only (~selftest.sh:2056-2093). container-init.sh:174-177 claims the signal path is handled; CR-77 shows it is not. Pairs with CR-77 as one work item.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-79 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-79","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"No criterion covers a signal arriving during the login swap","original_scope_note":"Signal assertions in the harness cover container stop only (~selftest.sh:2056-2093). container-init.sh:174-177 claims the signal path is handled; CR-77 shows it is not. Pairs with CR-77 as one work item.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:06:00 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-80","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"The restore staging file $cfg.dw-restore.$$ is a plaintext token copy that nothing cleans up","original_scope_note":"container-init.sh:196. Not named by the trap and not matched by the age sweep at :179 (-name .dw-tea-bak.*). A SIGKILL between the cp -p and the mv -f leaves it in ~/.config/tea/ permanently. Reproduced against the sweep verbatim: survivor config.yml.dw-restore.12345. Same directory and mode as the config it stages, so residue rather than new exposure.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-80 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-80","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"The restore staging file $cfg.dw-restore.$$ is a plaintext token copy that nothing cleans up","original_scope_note":"container-init.sh:196. Not named by the trap and not matched by the age sweep at :179 (-name .dw-tea-bak.*). A SIGKILL between the cp -p and the mv -f leaves it in ~/.config/tea/ permanently. Reproduced against the sweep verbatim: survivor config.yml.dw-restore.12345. Same directory and mode as the config it stages, so residue rather than new exposure.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:06:03 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-81","decision":"D-PO-61-9-1","category":"pre-existing","summary":"The forge token is passed in argv, and /proc/<pid>/cmdline is world-readable","original_scope_note":"dw.sh:400, :424, :647 and container-init.sh:190. Reproduced with a fake token: cmdline perms 444, token plainly visible, hidepid not set. Pre-existing — `git log -S` dates it to a046148 (WU-61-3-2), before this diff. Mitigated by a single-operator workstation and a least-privilege token (asserted by assert_token_least_privilege). Recorded because push_file goes to real trouble to keep the same token off docker exec -e and out of argv, so the two paths disagree about their own threat model.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-81 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-81","decision":"D-PO-61-9-1","category":"pre-existing","summary":"The forge token is passed in argv, and /proc/<pid>/cmdline is world-readable","original_scope_note":"dw.sh:400, :424, :647 and container-init.sh:190. Reproduced with a fake token: cmdline perms 444, token plainly visible, hidepid not set. Pre-existing — `git log -S` dates it to a046148 (WU-61-3-2), before this diff. Mitigated by a single-operator workstation and a least-privilege token (asserted by assert_token_least_privilege). Recorded because push_file goes to real trouble to keep the same token off docker exec -e and out of argv, so the two paths disagree about their own threat model.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:06:07 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-82","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"The stated reason for placing the login snapshot beside the config is factually wrong","original_scope_note":"container-init.sh:166-171 argues the snapshot must not live in /tmp because /tmp is the container writable layer, outside both volumes dw.sh tracks. But create_container mounts exactly two volumes (dw.sh:81, :691-693), and ~/.config/tea/ is a sibling of .claude — so it is the writable layer too, exactly like /tmp. The placement is still right; the justification is not, and a wrong justification is what a later reader will reason from.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-82 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-82","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"The stated reason for placing the login snapshot beside the config is factually wrong","original_scope_note":"container-init.sh:166-171 argues the snapshot must not live in /tmp because /tmp is the container writable layer, outside both volumes dw.sh tracks. But create_container mounts exactly two volumes (dw.sh:81, :691-693), and ~/.config/tea/ is a sibling of .claude — so it is the writable layer too, exactly like /tmp. The placement is still right; the justification is not, and a wrong justification is what a later reader will reason from.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:06:10 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-83","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"push_file creates directories outside the home bound before refusing","original_scope_note":"dw.sh:274-276: mkdir -p runs before the resolved-parent bound is applied. Reproduced: rc=1 and no content written, but out7/created/deep now exists. Empty directories only; cosmetic.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-83 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-83","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"push_file creates directories outside the home bound before refusing","original_scope_note":"dw.sh:274-276: mkdir -p runs before the resolved-parent bound is applied. Reproduced: rc=1 and no content written, but out7/created/deep now exists. Empty directories only; cosmetic.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:06:14 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-84","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"A truncated input stream is renamed into place and reported as success (script level only; not reproduced end-to-end)","original_scope_note":"dw.sh:303-304: `cat > $t; mv -f $t $DW_DEST`, and cat returns 0 when the producer closes early. Reproduces at script level through a fifo (22 bytes installed, rc=0) but the reviewer could NOT reproduce a short stream through `docker exec -i` with a plain-file redirect, so the reachable half is unproven and is labelled as such. Every real caller is covered downstream anyway (jq -e plus a live probe; tea login add plus probe_forge_token; git ls-remote). A size check after the rename would close it cheaply.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-84 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-84","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"A truncated input stream is renamed into place and reported as success (script level only; not reproduced end-to-end)","original_scope_note":"dw.sh:303-304: `cat > $t; mv -f $t $DW_DEST`, and cat returns 0 when the producer closes early. Reproduces at script level through a fifo (22 bytes installed, rc=0) but the reviewer could NOT reproduce a short stream through `docker exec -i` with a plain-file redirect, so the reachable half is unproven and is labelled as such. Every real caller is covered downstream anyway (jq -e plus a live probe; tea login add plus probe_forge_token; git ls-remote). A size check after the rename would close it cheaply.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:06:17 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-85","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"The container-side flock has no timeout and prints nothing while waiting; fd 9 is inherited by children","original_scope_note":"container-init.sh:127-128. Verified in the real image: a child sees fd 9, and a child outliving the shell keeps the lock even after `exec 9>&-`. Nothing in the locked function daemonizes, so that half is theoretical. The practical half is that `flock 9` blocks forever with no -w and no message, so an `up` meeting a stuck lock hangs unexplained — unlike the host lock at dw.sh:124-142, which warns, times out at 600s and names the remedy.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-85 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-85","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"The container-side flock has no timeout and prints nothing while waiting; fd 9 is inherited by children","original_scope_note":"container-init.sh:127-128. Verified in the real image: a child sees fd 9, and a child outliving the shell keeps the lock even after `exec 9>&-`. Nothing in the locked function daemonizes, so that half is theoretical. The practical half is that `flock 9` blocks forever with no -w and no message, so an `up` meeting a stuck lock hangs unexplained — unlike the host lock at dw.sh:124-142, which warns, times out at 600s and names the remedy.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:06:21 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-86","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"CR-68 dangling-git case is a duplicate of files-no-repo and buys no coverage","original_scope_note":"selftest.sh:2599-2602. The seed makes .git a symlink to a non-existent path, so `[ -e .git ]` (which follows symlinks) is false and the guard takes the same branch as files-no-repo, printing an identical reason — confirmed in the baseline log at CR-68.log:6 and :18. Redundant rather than vacuous: one mutation kills both, and it costs a container per run. If the intent was a repository git cannot open, the shape needs a .git that EXISTS and is corrupt, which reaches G rev-parse --is-inside-work-tree.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-86 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-86","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"CR-68 dangling-git case is a duplicate of files-no-repo and buys no coverage","original_scope_note":"selftest.sh:2599-2602. The seed makes .git a symlink to a non-existent path, so `[ -e .git ]` (which follows symlinks) is false and the guard takes the same branch as files-no-repo, printing an identical reason — confirmed in the baseline log at CR-68.log:6 and :18. Redundant rather than vacuous: one mutation kills both, and it costs a container per run. If the intent was a repository git cannot open, the shape needs a .git that EXISTS and is corrupt, which reaches G rev-parse --is-inside-work-tree.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:06:24 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-87","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"cr68_case scratch issue numbers collide between shapes","original_scope_note":"selftest.sh:2637 derives the number from two cksum characters: in the baseline run dangling-git and clean both got 9776032, ignored and unreadable both got 9776026. Harmless today because cases run strictly in sequence and each removes its volume first, but the criterion can never be parallelised and a future reordering that leaves a volume behind would have one shape inspecting another workspace. Cheap fix: append the shape name instead of a 2-char hash.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-87 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-87","decision":"D-PO-61-9-1","category":"in-scope-deferrable","summary":"cr68_case scratch issue numbers collide between shapes","original_scope_note":"selftest.sh:2637 derives the number from two cksum characters: in the baseline run dangling-git and clean both got 9776032, ignored and unreadable both got 9776026. Harmless today because cases run strictly in sequence and each removes its volume first, but the criterion can never be parallelised and a future reordering that leaves a volume behind would have one shape inspecting another workspace. Cheap fix: append the shape name instead of a 2-char hash.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:06:28 +00:00
Author
Owner
Copy link
{"status":"active","from_issue":"61","finding_ref":"CR-88","decision":"D-PO-61-9-1","category":"out-of-scope","summary":"A stray unlabelled dw--workspace volume from an earlier round is still on the box","original_scope_note":"docker volume ls shows dw--workspace created 2026-08-26, no labels. It cannot come from current dw.sh — require_issue (dw.sh:152-157) rejects an empty argument — so it is residue from a hand-run docker volume create in an earlier round. Housekeeping on the operator box, not a defect in the deliverable; recorded so it gets swept rather than puzzled over later.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-88 status=active decision=D-PO-61-9-1 --> ```json {"status":"active","from_issue":"61","finding_ref":"CR-88","decision":"D-PO-61-9-1","category":"out-of-scope","summary":"A stray unlabelled dw--workspace volume from an earlier round is still on the box","original_scope_note":"docker volume ls shows dw--workspace created 2026-08-26, no labels. It cannot come from current dw.sh — require_issue (dw.sh:152-157) rejects an empty argument — so it is residue from a hand-run docker volume create in an earlier round. Housekeeping on the operator box, not a defect in the deliverable; recorded so it gets swept rather than puzzled over later.","disposition_rationale":"Round-5 non-blocking finding, batched to #353 under D-PO-61-9-1. The round confirmed 5 blocking defects, which were fixed in place; everything else is real but recoverable or cosmetic and does not justify holding #61 in QA."} ```
jbr870 commented 2026-08-31 14:06:49 +00:00
Author
Owner
Copy link
{"status":"withdrawn","from_issue":"61","finding_ref":"CR-75","decision":"D-PO-61-9-1","withdrawn_reason":"Fixed in round 5, not deferred: CR-68 gained the 'unreadable-subdir' and 'submodule' shapes this finding asked for, and both are mutation-verified (deleting the guard each covers turns CR-68 red). Batched here by mistake in the same pass as the genuinely deferred findings."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-75 status=withdrawn decision=D-PO-61-9-1 --> ```json {"status":"withdrawn","from_issue":"61","finding_ref":"CR-75","decision":"D-PO-61-9-1","withdrawn_reason":"Fixed in round 5, not deferred: CR-68 gained the 'unreadable-subdir' and 'submodule' shapes this finding asked for, and both are mutation-verified (deleting the guard each covers turns CR-68 red). Batched here by mistake in the same pass as the genuinely deferred findings."} ```
jbr870 commented 2026-08-31 14:06:54 +00:00
Author
Owner
Copy link
{"status":"withdrawn","from_issue":"61","finding_ref":"CR-86","decision":"D-PO-61-9-1","withdrawn_reason":"Fixed in round 5, not deferred: the 'dangling-git' shape now writes a .git FILE pointing at a missing gitdir, so it survives the existence test and dies at the first real query — the distinct case this finding said it was missing. Batched here by mistake in the same pass as the genuinely deferred findings."}
<!-- folded-finding:v1 issue=353 from_issue=61 finding_ref=CR-86 status=withdrawn decision=D-PO-61-9-1 --> ```json {"status":"withdrawn","from_issue":"61","finding_ref":"CR-86","decision":"D-PO-61-9-1","withdrawn_reason":"Fixed in round 5, not deferred: the 'dangling-git' shape now writes a .git FILE pointing at a missing gitdir, so it survives the existence test and dies at the first real query — the distinct case this finding said it was missing. Batched here by mistake in the same pass as the genuinely deferred findings."} ```
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
interactive-skills
No results found.
Labels
Clear labels   
component:adapters

   
component:lifecycle

   
component:qa

   
component:setup

   
component:shared

   
component:worktrees

   
phase/accepted

   
phase/backlog

   
phase/deployed

   
phase/developing

   
phase/integrating

   
phase/planning

   
phase/qa

   
phase/requirements

   
phase/uat

   
priority:critical

   
priority:critical

   
priority:high

   
priority:high

   
priority:low

   
priority:low

   
priority:medium

   
priority:medium

   
type:bug

   
type:chore

   
type:docs

   
type:feature

   
type:infra

   
type:tech-debt
No labels
component:adapters
component:lifecycle
component:qa
component:setup
component:shared
component:worktrees
phase/accepted
phase/backlog
phase/deployed
phase/developing
phase/integrating
phase/planning
phase/qa
phase/requirements
phase/uat
priority:critical
priority:critical
priority:high
priority:high
priority:low
priority:low
priority:medium
priority:medium
type:bug
type:chore
type:docs
type:feature
type:infra
type:tech-debt
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
jbr870/devwork-skills#353
No description provided.