Skip to content

Bug-hunt round 45: atomicConvert's existing-file mode reset - #64

Merged
REPPL merged 3 commits into
mainfrom
bughunt-45
Aug 15, 2026
Merged

Bug-hunt round 45: atomicConvert's existing-file mode reset#64
REPPL merged 3 commits into
mainfrom
bughunt-45

Conversation

@REPPL

@REPPL REPPL commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Round 45 of the autonomous bug-hunt loop (state tracked on #24).

Confirmed findings

Substantive (1)

transcribe.atomicConvert resets an existing audio.wav's mode instead of preserving itinternal/transcribe/ffmpeg.go (the pre-fix atomicConvert) chmod'd its temp file to the umask-masked default unconditionally before renaming it over out, even when out already existed with a deliberately different mode.

A direct ffmpeg write over an existing file leaves that file's mode untouched — open(2)'s mode argument is only consulted on O_CREAT — so the rename-into-place path diverged from the very behaviour its own comment claimed to reproduce ("a direct ffmpeg write would have created…").

Reachable on an ordinary transcribe -audio re-run over a session whose audio.wav an operator had tightened to 0600 (a privacy-conscious operator hardening the raw voice recording, or a session directory copied from a machine with a different umask): the recording came back group/world-readable. The reverse also occurred — a deliberately widened 0666 silently narrowed to 0644.

internal/session/session.go's WriteFileAtomicNoFollow already makes this exact guarantee for the offset sidecar (documented explicitly: "an operator-tightened sidecar stays tightened and a deliberately widened one is not silently narrowed"), so audio.wav — the more privacy-sensitive of the two — was the outlier among the session's atomic writers.

Fix: atomicConvert now Lstats out first and, when it is already a regular file, preserves its mode across the rename instead of reapplying the umask-derived default (which still applies, unchanged, when out does not yet exist).

New TestAtomicConvertPreservesExistingMode covers both directions (0600 preserved, 0666 preserved) — confirmed to fail before this change and pass after.

Considered and rejected

  • internal/demo/assets/index.html's rrweb CDN <script> tag lacking a Subresource Integrity hash. Real and unmitigated — every other external artefact this project consumes (GitHub Actions, gitleaks, macOS ffmpeg, release tarballs) is hash- or signature-pinned, and this tag isn't. Both independent adversarial reviews killed it as an in-round fix, not as a non-issue: round 9 already disclosed the CDN fetch and explicitly deferred the vendoring decision as a standing, human-owned tradeoff (.abcd/work/DECISIONS.md); the tag is pinned to an exact, immutable npm version rather than a floating one, materially narrowing the residual risk to a jsDelivr-side compromise; and computing a trustworthy SRI hash requires network access to the CDN, which this environment's proxy blocks — an unverified hash risks a silent false pin, worse than no pin at all, and a wrong hash fails invisibly (the page's own if (window.rrweb) fallback swallows a blocked script with no CI coverage). Recorded for whoever takes the vendoring/pinning decision; not fixed here.
  • .abcd/development/specs/open/spc-2-analysis-findings.md's finding-schema table omitting the 64-id evidence cap that every shipped sibling reference page (docs/reference/session-directory.md, docs/reference/cli.md, .abcd/development/brief/05-internals/02-schemas.md, .abcd/development/brief/04-surfaces/06-analyze.md) states. Split adversarial verdict: one reviewer found the omission is not an isolated unswept gap but one of at least five ingest-enforced rules the table has drifted from since it predates them, and that spc-2 itself designates 05-internals/02-schemas.md as the canonical successor table under its own schema-move note; the other reviewer judged the specific omission real but nitpick-tier. Discarded per the loop's own "when in doubt, discard" tie-breaking rule.

Verification

go build, gofmt -l ., go vet ./..., go test ./..., go test -race ./... all clean; pipeline smoke (merge + report against a scratch copy of examples/sample-session) succeeded; sh -n install.sh && bash -n install.sh clean. Working tree otherwise untouched.


Assisted-by: Claude:claude-sonnet-5

REPPL added 3 commits August 15, 2026 21:30
atomicConvert chmod'd its temp file to the umask-masked default
unconditionally before renaming it over out, even when out already
existed with a deliberately different mode. A direct ffmpeg write over
an existing file leaves that file's mode untouched (open(2)'s mode
argument is only consulted on O_CREAT), so the rename path diverged
from the very behaviour its own comment claimed to reproduce.

Reachable on an ordinary `transcribe -audio` re-run over a session
whose audio.wav an operator had tightened to 0600, or one copied from
a machine with a different umask: the recording came back
group/world-readable, and the reverse (0666 silently narrowed to
0644) also occurred. session.WriteFileAtomicNoFollow already makes
this exact guarantee for the offset sidecar; atomicConvert now Lstats
out first and preserves its mode across the rename when it is already
a regular file.

New TestAtomicConvertPreservesExistingMode covers both directions
(confirmed to fail before this change and pass after).

Assisted-by: Claude:claude-sonnet-5
Assisted-by: Claude:claude-sonnet-5
DECISIONS.md described the spc-2 split-verdict discard as a second
"refuted" finding; it was discarded on a tie, not refuted outright.
The atomicConvert comment's O_CREAT phrasing faintly implied the
opposite of the point being made about which case reapplies the
umask-masked default. Both raised by this round's own adversarial
PR review.

Assisted-by: Claude:claude-sonnet-5
@REPPL
REPPL merged commit dc85428 into main Aug 15, 2026
6 checks passed
@REPPL
REPPL deleted the bughunt-45 branch August 15, 2026 21:40
@REPPL REPPL mentioned this pull request Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant