[codex] Add encrypted backup file bundles#38
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 19, 2026, 2:12 PM ET / 18:12 UTC. Summary Reproducibility: yes. from source inspection: a root that is itself a directory symlink passes the initial Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Reject or safely resolve symlinked collection roots before any manifest write, keep the file-bundle manifest additive, remove the release-owned changelog edit, and require redacted real backup/restore plus downstream compatibility proof before merge. Do we have a high-confidence way to reproduce the issue? Yes, from source inspection: a root that is itself a directory symlink passes the initial Is this the best way to solve the issue? No; the provider-neutral feature belongs in crawlkit, but the patch should fail visibly or safely resolve symlinked roots before writing a manifest and pruning file objects. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 797628aa9708. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f70d9cb225
ℹ️ 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".
| if entry.Type()&os.ModeSymlink != 0 { | ||
| if entry.IsDir() { | ||
| return filepath.SkipDir | ||
| } | ||
| return nil |
There was a problem hiding this comment.
When the configured root itself is a symlink to a directory, filepath.WalkDir visits only that symlink, entry.IsDir() is false, and this branch returns nil, so CollectFiles returns an empty file list. A downstream backup configured to a symlinked media/cache directory would then write a manifest with no file entries and remove the previously tracked file bundle blobs as stale. Handle source == root separately by resolving the root or returning an error rather than silently skipping it.
Useful? React with 👍 / 👎.
| if !info.Mode().IsRegular() { | ||
| return nil, fmt.Errorf("backup file is not regular: %s", file.Source) | ||
| } | ||
| tmpPath, hashValue, size, encryptedSize, err := encryptFileTemp(ctx, file.Source, cfg.Repo, cfg.Recipients) |
There was a problem hiding this comment.
For unchanged files from the previous manifest, and for duplicate content later in the same snapshot, this call still writes a full gzip+age temp blob before the written/oldByHash reuse checks below discard it. On a large media backup, an otherwise no-op run can fail with ENOSPC or take hours because it needs room and CPU for a second encrypted copy of each unchanged file. Compute the plaintext hash/size first and only encrypt when the hash is not already reusable.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9c1c822ef
ℹ️ 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".
| out = append(out, entry) | ||
| continue | ||
| } | ||
| shard := path.Join("data/files", hashValue[:2], hashValue+".gz.age") |
There was a problem hiding this comment.
When reuseEncrypted is false because the recipient set changed, an unchanged file still maps to the same hash-only data/files/<hash>.gz.age path, so the rename below overwrites the blob referenced by the old manifest before the new manifest is safely written. If the process crashes or a later write fails during recipient rotation, the previous manifest remains on disk but its file object is now encrypted for the new recipients; the row-shard path avoids this by using a versioned path when !reuseEncrypted.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
P2
|
Validation complete:
Review-driven fixes covered path privacy, source/restore symlink confinement, encrypted-index namespace collisions, legacy cleanup compatibility, recipient-only writes, Windows test portability, and canonical slash-only shard paths. |
1 participant
Summary
Validation
go test ./...go test -race ./...go vet ./...