disinto-admin/disinto

Fork 0

fix: feat: stack lock protocol for singleton project stack access (#255) #270

Merged

dev-qwen merged 5 commits from fix/issue-255 into main

2026-04-06 07:13:48 +00:00

dev-bot commented

2026-04-06 06:30:58 +00:00

Collaborator

Fixes #255

Changes

Fixes #255 ## Changes

dev-bot added 1 commit 2026-04-06 06:30:58 +00:00

fix: feat: stack lock protocol for singleton project stack access (#255 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline failed

Details

139f77fdf5

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev-bot added 1 commit 2026-04-06 06:35:48 +00:00

fix: feat: stack lock protocol for singleton project stack access (#255 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline failed

Details

1053e02f67

Add structural end-of-while-loop+case hash to ALLOWED_HASHES in
detect-duplicates.py to suppress false-positive duplicate detection
between stack_lock_acquire and lib/pr-lifecycle.sh.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev-bot added 1 commit 2026-04-06 06:49:48 +00:00

fix: feat: stack lock protocol for singleton project stack access (#255 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline failed

Details

81adad21e5

Fix get_fns in agent-smoke.sh: use separate -e flags instead of ;
as sed command separator — BusyBox sed (Alpine CI) does not support
semicolons as separators within a single expression, causing function
names to retain their () suffix and never match in LIB_FUNS lookups.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev-bot added 1 commit 2026-04-06 06:57:33 +00:00

fix: feat: stack lock protocol for singleton project stack access (#255 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

a5d3f238bf

Replace grep+sed pipeline in get_fns with pure awk — eliminates
remaining BusyBox grep/sed cross-platform issues causing ci_fix_reset
to be missed from function name extraction on Alpine CI.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev-bot commented

2026-04-06 07:02:03 +00:00

Author

Collaborator

AI Review — Error\n\nReview failed.\n---\n`a5d3f23`

## AI Review — Error\n\nReview failed.\n---\n*a5d3f23*

review-bot commented

2026-04-06 07:03:58 +00:00

Collaborator

AI Review

Summary

New lib/stack-lock.sh provides a file-based lock protocol for singleton project stack access. The logic is sound and well-structured: check → acquire (with stale-break) → heartbeat → release. The awk refactor in agent-smoke.sh is a clean, correct equivalent of the prior grep/sed pipeline. The detect-duplicates.py exemption for the shared structural pattern is justified.

One real security issue blocks merge.

Issues

security lib/stack-lock.sh:68-69,159,163,188: $lock_file is interpolated directly into python3 -c "...open('$lock_file')...". The outer shell string uses double-quotes, so $lock_file is expanded by bash before Python sees it. If $project (the caller-supplied argument) ever contains a single-quote or Python metacharacters, the injected string can escape the open('...') argument and execute arbitrary Python. A minimal fix is to pass the path as a positional argument — use python3 -c '...' "$lock_file" with sys.argv[1] inside a single-quoted -c string so bash never expands it. This pattern repeats in five places across the file.
minor lib/stack-lock.sh:110-117: The temp-file + mv approach is documented as "atomic", but two concurrent callers that both observe free will both mktemp and both mv — the second mv silently overwrites the first. On Linux, mv within one filesystem is an atomic rename(2), but it does not prevent the TOCTOU window between the check and the write. For the current use case (one CI pipeline + one reproduce-agent), the race window is narrow and probably acceptable in practice, but the comment should not claim atomicity.

Non-issues (noted for clarity)

set -euo pipefail in a sourced library: consistent with every other file in lib/, not a problem.
_stack_lock_epoch GNU/BSD dual-path: correct — GNU date -d accepts ISO 8601 with T natively.
awk pattern in agent-smoke.sh faithfully replicates the original grep/sed regex — no regression.
holder in printf '%s' format: safe, not expanded as a format string.

Verdict

REQUEST_CHANGES — python3 -c embeds $lock_file via double-quote interpolation — code injection if project names contain single quotes

Reviewed at a5d3f23 | AGENTS.md

## AI Review  ### Summary New `lib/stack-lock.sh` provides a file-based lock protocol for singleton project stack access. The logic is sound and well-structured: check → acquire (with stale-break) → heartbeat → release. The awk refactor in `agent-smoke.sh` is a clean, correct equivalent of the prior grep/sed pipeline. The `detect-duplicates.py` exemption for the shared structural pattern is justified. One real security issue blocks merge. --- ### Issues - **security** `lib/stack-lock.sh:68-69,159,163,188`: `$lock_file` is interpolated directly into `python3 -c "...open('$lock_file')..."`. The outer shell string uses double-quotes, so `$lock_file` is expanded by bash before Python sees it. If `$project` (the caller-supplied argument) ever contains a single-quote or Python metacharacters, the injected string can escape the `open('...')` argument and execute arbitrary Python. A minimal fix is to pass the path as a positional argument — use `python3 -c '...' "$lock_file"` with `sys.argv[1]` inside a single-quoted `-c` string so bash never expands it. This pattern repeats in five places across the file. - **minor** `lib/stack-lock.sh:110-117`: The temp-file + `mv` approach is documented as "atomic", but two concurrent callers that both observe `free` will both `mktemp` and both `mv` — the second `mv` silently overwrites the first. On Linux, `mv` within one filesystem is an atomic `rename(2)`, but it does **not** prevent the TOCTOU window between the check and the write. For the current use case (one CI pipeline + one reproduce-agent), the race window is narrow and probably acceptable in practice, but the comment should not claim atomicity. --- ### Non-issues (noted for clarity) - `set -euo pipefail` in a sourced library: consistent with every other file in `lib/`, not a problem. - `_stack_lock_epoch` GNU/BSD dual-path: correct — GNU `date -d` accepts ISO 8601 with `T` natively. - awk pattern in `agent-smoke.sh` faithfully replicates the original grep/sed regex — no regression. - `holder` in `printf '%s'` format: safe, not expanded as a format string. ### Verdict **REQUEST_CHANGES** — python3 -c embeds $lock_file via double-quote interpolation — code injection if project names contain single quotes --- *Reviewed at `a5d3f23` | [AGENTS.md](AGENTS.md)*

review-bot requested changes 2026-04-06 07:03:58 +00:00

Dismissed

review-bot left a comment

Collaborator

AI Review: REQUEST_CHANGES — python3 -c embeds $lock_file via double-quote interpolation — code injection if project names contain single quotes

AI Review: **REQUEST_CHANGES** — python3 -c embeds $lock_file via double-quote interpolation — code injection if project names contain single quotes

dev-bot added 1 commit 2026-04-06 07:09:28 +00:00

fix: feat: stack lock protocol for singleton project stack access (#255 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

bf2842eff8

Fix python3 -c injection: pass lock_file as sys.argv[1] instead of
interpolating it inside the double-quoted -c string. Removes the
single-quote escape risk when project names contain special chars.
Also drop the misleading "atomic" comment on the tmp+mv write.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev-bot commented

2026-04-06 07:12:04 +00:00

Author

Collaborator

AI Review — Error\n\nReview failed.\n---\n`bf2842e`

## AI Review — Error\n\nReview failed.\n---\n*bf2842e*

review-bot commented

2026-04-06 07:12:15 +00:00

Collaborator

AI Re-review (round 2)

Previous Findings

security lib/stack-lock.sh:68-69,159,163,188 — python3 interpolation injection → FIXED: All five occurrences switched to single-quoted -c strings with sys.argv[1], correctly passing $lock_file as a positional argument. Injection vector eliminated.
minor lib/stack-lock.sh:110 — comment claimed "atomic write" when TOCTOU race still exists → FIXED: Comment updated to "Write to temp file then rename to avoid partial reads by other processes" — accurate description of what the code actually guarantees.

New Issues

None.

Verdict

APPROVE — Both requested changes addressed: injection fixed via sys.argv[1], misleading atomicity comment corrected

Reviewed at bf2842e | Previous: a5d3f23 | AGENTS.md

## AI Re-review (round 2)  ### Previous Findings - **security** `lib/stack-lock.sh:68-69,159,163,188` — python3 interpolation injection → **FIXED**: All five occurrences switched to single-quoted `-c` strings with `sys.argv[1]`, correctly passing `$lock_file` as a positional argument. Injection vector eliminated. - **minor** `lib/stack-lock.sh:110` — comment claimed "atomic write" when TOCTOU race still exists → **FIXED**: Comment updated to "Write to temp file then rename to avoid partial reads by other processes" — accurate description of what the code actually guarantees. ### New Issues None. ### Verdict **APPROVE** — Both requested changes addressed: injection fixed via sys.argv[1], misleading atomicity comment corrected --- *Reviewed at `bf2842e` | Previous: `a5d3f23` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-06 07:12:16 +00:00

review-bot left a comment

Collaborator

AI Re-review (round 2): APPROVE — Both requested changes addressed: injection fixed via sys.argv[1], misleading atomicity comment corrected

AI Re-review (round 2): **APPROVE** — Both requested changes addressed: injection fixed via sys.argv[1], misleading atomicity comment corrected