Skip to content

Make sensitive rewrites and prune cleanup atomic#129

Merged
brucehart merged 7 commits into
pr-127-feedbackfrom
pr-128-feedback
Jul 13, 2026
Merged

Make sensitive rewrites and prune cleanup atomic#129
brucehart merged 7 commits into
pr-127-feedbackfrom
pr-128-feedback

Conversation

@brucehart

Copy link
Copy Markdown
Owner

Summary

  • stage redaction and PII output databases until the rewrite and sensitive-data cleanup complete
  • make PII entry rewrites transactional and defer prune filesystem deletion until after database commit
  • preserve external files that remain referenced by surviving blobs
  • add rollback, failed-publication, forced-overwrite, and shared-path regressions

Validation

  • cargo test --all-features --locked (201 passed)
  • cargo clippy --all-features --all-targets --locked (passes; existing warnings only)
  • cargo check --no-default-features --locked
  • cargo check --no-default-features --features parquet --locked
  • git diff --check

Context

Addresses the must-fix findings from the recursive review of #128: #128 (comment)

Stacked on #128 so the original security-hardening PR remains unchanged.

Stage output database copies until redaction completes so failures preserve any existing destination. Wrap PII entry rewrites in a single transaction, defer prune filesystem deletion until after commit, and retain external files still referenced by surviving blobs. Add regressions for rollback, publication failure, overwrite, and shared external paths.
@brucehart

Copy link
Copy Markdown
Owner Author

@codex review

@brucehart

Copy link
Copy Markdown
Owner Author

Recursive review summary

Blockers

None.

Must-fix

  • [medium] Fail closed when decoding surviving external paths. src/commands/prune.rs collects the rows with filter_map(|row| row.ok()), so a non-TEXT SQLite value is silently omitted from external_paths_in_use. SQLite allows dynamic types even in this TEXT-declared column. I reproduced a surviving blob with the same path stored as BLOB: prune committed successfully, deleted the shared file, and left the surviving blob reference in the database. Propagate row-decoding errors before commit instead; malformed/untrusted databases must preserve both the transaction and filesystem. Affected code

Nice-to-have

None.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e987692da9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/commands/util.rs Outdated
Comment thread src/commands/pii.rs
Propagate SQLite decoding errors while protecting surviving external blob paths so malformed databases roll back before filesystem deletion. Clean rollback-journal sidecars with staged databases and add regressions for both cases.
@brucehart

Copy link
Copy Markdown
Owner Author

Addressed the must-fix finding in stacked PR #130: #130

Prune now propagates surviving-path decoding failures and rolls the transaction back before filesystem deletion. The regression reproduces a dynamically typed SQLite BLOB path and verifies the shared file, import, and orphan blob are all preserved. The same change also cleans rollback-journal sidecars for staged databases. Full validation passes (204 tests).

Back up an existing destination database and its SQLite sidecars as a recoverable set before publishing staged output. Move orphaned-blob cleanup into the PII and redact transactions so cleanup errors roll back every logical rewrite. Add restore, replacement, and cleanup-failure regressions.
Simulate a failed final rename and verify the prior destination database and every SQLite sidecar are restored without leaving a backup directory.
Propagate malformed entry hashes, orphan blob hashes, and pruned external paths instead of silently skipping them. Add regressions proving malformed pruned paths and entry hashes roll back the import transaction.
Fail closed on every prune row decode
Fail closed on malformed prune paths
@brucehart
brucehart merged commit 4f20616 into pr-127-feedback Jul 13, 2026
4 checks passed
@brucehart
brucehart deleted the pr-128-feedback branch July 13, 2026 21:55
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