Skip to content

fix: preserve readable staging safety chain - #191

Open
junior-ricon wants to merge 3 commits into
mainfrom
junior/staging-safety-chain
Open

fix: preserve readable staging safety chain#191
junior-ricon wants to merge 3 commits into
mainfrom
junior/staging-safety-chain

Conversation

@junior-ricon

Copy link
Copy Markdown
Contributor

Summary

  • require the complete current Notes pre-commit hook chain before notes stage mutates the index
  • warn after notes unlock when required hook protection is missing or stale
  • clear managed opaque paths' skip-worktree state and stage them with sparse-aware Git semantics
  • preserve dry-run/no-change behavior, scoped staging, encryption filters, and normal commit cycles

Closes #151.
Closes #152.

Validation

  • 449 BATS tests and 9 Python tests
  • all eight configured Codebase lints
  • focused stage, encryption, hook, suppression, obfuscation, and commit suites
  • disposable current-source reproductions: missing hooks refuse before index mutation; a skipped opaque ID commits through auto-obfuscation
  • doctor and README generation/check
  • Bash syntax and git diff --check
  • Fold pre-commit and pre-push

@johnson-ricon johnson-ricon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review of exact head e9d7200494a63b32a5a712de0503935f0364246d against exact base e16cd6c9bf271a595ed4833cc6ff02e75d461d49.

Change request

lib/hooks.sh:103-111 does not prove that Git will execute the required chain. It hardcodes .git/hooks, even when core.hooksPath selects another active directory, and it accepts any executable dispatcher that merely contains the text pre-commit.d.

I reproduced both paths in disposable repositories. In one, Notes installed and inspected inactive .git/hooks while Git used .githooks. In the other, pre-commit mentioned pre-commit.d in a comment and exited zero without running fragments. In each case notes stage alpha.md succeeded and the next normal commit added readable notes/alpha.md, so the #152 fail-closed invariant still has a bypass.

Focused fix-it PR: #192. It resolves and installs into Git's active hook directory, requires the current Notes dispatcher exactly, and verifies refusal before index mutation plus repair and an opaque-name commit cycle.

Stress tests

The original head passed the focused stage (25), commit (13), encrypt (25), obfuscation-hooks (24), obfuscate (40), and status-suppression (10) suites. I also exercised a real non-cone sparse checkout with a skipped unselected opaque ID and a real Git Crypt clean filter. The selected opaque ID committed encrypted, cat-file --filters restored the expected plaintext, the unselected ID remained skip-worktree, and no readable path entered the commit.

I checked missing and stale required fragments, refusal before index mutation, unlock warnings, dry-run, scoped staging, no-change control flow, assume-unchanged and skip-worktree handling, and the normal stage/commit/deobfuscate lifecycle. Hosted macOS and Ubuntu checks are green on the exact reviewed head, and GitHub reports it cleanly mergeable.

The sparse, filter, suppression, scope, and lifecycle changes otherwise held under review. I found no second blocker. Requesting changes for the active-dispatcher bypass above.

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.

Fail closed when staging readable notes without installed hooks Auto-obfuscation cannot stage skip-worktree note path

2 participants