Repository navigation
Fix R01-R06: remaining findings from the 2026-09-30 re-audit - #12
Merged
Merged
Conversation
escapeGitattributesPattern replaced a space with backslash-space, but git's own gitattributes line parser still splits on that whitespace — verified directly against `git check-attr`. Quote the whole pattern instead (the same C-quoted form git already honors), escaping only what the quoting layer itself needs escaped. Also closes the test gap the audit flagged: the new integration test writes the produced line into a real .gitattributes and asks git whether it actually resolves the filter, instead of only checking the helper's string output. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
DirtyFiles only compared the working tree against HEAD, so a file whose worktree content was restored back to HEAD while the index still held a different, staged version was reported as clean. CheckoutAll's subsequent `git checkout HEAD -- <path>` then silently discarded that staged change. DirtyFiles now also diffs the index against HEAD and returns the union, so either kind of drift is caught before it's overwritten. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The auto-generated password was only ever printed at the very end of rotate, after registering the new key, saving the config, and re-encrypting everything under it. A failure at any of those steps left the new key already active with its password never shown. Print it immediately once it exists, before anything that could still fail. Failures partway through now also point out that nothing has been committed yet, so `git checkout -- .` (or `git reset --hard HEAD`) fully undoes the attempt, and re-running `nebel rotate` converges the rest instead of leaving anything half-migrated. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
writeKeyFile truncated the key file in place, so an interruption mid-write could leave it empty or partial, and Set's read-modify-write had no synchronization, so two concurrent calls could silently lose an update. writeKeyFile now writes to a temp file in the same directory and renames it into place, an atomic replace a reader never observes partially. Set is now also guarded by an in-process mutex. Cross-process locking remains out of scope, documented as a deliberate trade-off rather than a silent gap, to avoid a unix-only flock on a project that also supports Windows. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
LooksEncrypted only checked the ENC[...] delimiters and a recognized algorithm name, so a plaintext value like ENC[AES256_SIV,not-a-real-tag] was treated as already-encrypted and left uncommitted-as-plaintext by Clean instead of being encrypted. LooksEncrypted now structurally validates every field (key version, base64 data, value type), rejecting anything malformed, while still accepting a keyless legacy pre-rotation whole-file tag with no key field at all, since tag.Parse itself requires one and would otherwise have broken that still-supported legacy shape. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cleanValues encrypted a scalar's plaintext without checking whether it could actually be restored later, so a YAML string containing an unescapable control character, or a JSON integer outside int64's range, was accepted and committed as ciphertext that permanently fails on smudge. cleanValues now runs the same validTypeLiteral and handler.Render checks smudgeValues already runs on the way out, up front on the plaintext before it's ever sealed, so an unrestorable value is a rejected clean instead of unrecoverable ciphertext. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
MuffinUser
marked this pull request as ready for review
September 30, 2026 08:23
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes all six remaining findings (R01-R06) from
audits/2026-09-30-repository-reanalysis.md, verified independently before implementation:escapeGitattributesPatternbackslash-escaped spaces, which git's gitattributes parser doesn't honor as an escape; it now quotes the whole pattern instead (verified against realgit check-attr).DirtyFilesonly diffed the worktree against HEAD, missing a change that exists only in the index; it now also diffs the index against HEAD.rotate's auto-generated password was only shown at the very end; it's now shown immediately after generation, before any state that could still fail, and failure messages now point at git-based recovery instead of leaving the user guessing.os.WriteFilewith no locking; it's now written atomically (temp file + rename) with an in-process mutex aroundSet's read-modify-write.LooksEncryptedaccepted anyENC[<known-algo>,...]-shaped string regardless of what followed; it now structurally validates every field, while still accepting the legacy keyless whole-file tag shape.Cleancould encrypt a value thatSmudgecould never render back (an unescapable control character, or a JSON integer outside int64's range); it now runs the same validationSmudgealready runs, up front, before encrypting.Test plan
go build ./...go vet ./...go test ./... -count=1(all 9 packages pass)🤖 Generated with Claude Code