Skip to content

fix: preserve CAS integrity and recover missing chunks - #14

Merged
aoruLola merged 13 commits into
fix/security-hardening-batchfrom
fix/cas-integrity-review-20260920
Sep 20, 2026
Merged

aoruLola merged 13 commits into
fix/security-hardening-batchfrom
fix/cas-integrity-review-20260920

Conversation

@aoruLola

@aoruLola aoruLola commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

First implementation batch from the maintainer-requested HyperTide engineering review: harden CAS publication and recovery, reconcile chunk metadata with physical storage, repair stacked-PR CI coverage, and document a prioritized improvement plan.

Stacked on #13, which is stacked on #12. Existing upstream work is preserved and is not attributed to this PR. main has not been modified, and nothing has been merged or deployed.

Engineering review and improvement plan

Findings fixed

Priority Problem Fix
P1 Concurrent CAS writers reused temp/<hash>, permitting truncation and shared-inode mutation around publication. UUID-qualified private staging files created with create_new; handles are released before publication and each operation cleans only its own staging file.
P1 Equal-size corrupt CAS objects were accepted as dedup hits. Existing content is BLAKE3-verified through a bounded 64 KiB buffer before deduplication succeeds.
P1 Wrong-size/corrupt objects could be discarded before a replacement was safely staged, and rename failure trusted destination existence alone. Preserve the old object until replacement publication and verify a competing destination before acknowledging success.
P1 Corrective CAS uploads failed on Windows because rename cannot replace an existing destination. Shared cross-platform replacement helper uses ReplaceFileW for an existing Windows destination; version-state persistence uses the same helper.
P1 PostgreSQL chunk rows could survive a lost CAS file and suppress the retransmission needed for recovery. Require both metadata and physical CAS presence before telling clients they can skip an upload.
P2 Missing chunk metadata short-circuited the physical storage check, hiding storage failures. Always evaluate storage.exists before combining metadata/content presence.
P2 Opening an unexpected FIFO at a CAS hash path can block on Unix before file-type validation. Inspect with symlink_metadata before open, reject non-regular entries, then recheck the opened handle before hashing.
P2 Reconciliation/existence checks treated FIFO, directory, or symlink entries as valid CAS objects. Define CAS existence centrally as “regular file exists”; special entries now surface as storage inconsistencies, and retrieve uses the same guard.
P2 CI checked only PRs targeting main, so stacked PRs did not receive equivalent checks. Run CI for all pull-request base branches.
P2 Production Compose smoke omitted required TRUSTED_PROXY_CIDRS. Add loopback-only trust for the CI direct-port fixture without weakening production validation.

Public API payloads, database schemas, CAS identities, and the object layout remain unchanged. No dependency was added.

Verified results

Final reviewed head: 4b14a6605a647b87c56ecf4ea40a75e0128e0c9f.

GitHub Actions CI run 35483872323 checked this PR head against base 7812c58c71c201c765bac098a87f5d0871115961.

Check Result
cargo check --workspace Passed
cargo fmt --all -- --check Passed
cargo clippy --workspace -- -D warnings Passed
cargo test --workspace Passed: 42 CLI + 99 server = 141 tests, 0 failed, 0 ignored
Windows CAS Repair Passed on windows-latest: 6/6 targeted tests
Runtime Smoke Passed
Compose Smoke Passed
Backup Restore Smoke Passed, including restore into a fresh Compose stack

Windows verification explicitly executed and passed both:

  • store_repairs_same_size_corruption
  • store_repairs_wrong_size_corruption

The Linux test run also explicitly passed:

  • fifo_object_path_is_rejected_before_read_open
  • storage_errors_propagate_even_when_chunk_is_unindexed
  • exists_and_retrieve_reject_fifo_object_entries

The PR now contains thirteen review-driven regression tests across CAS publication/repair and chunk reconciliation. The chunk unit tests use real temporary storage plus an injected metadata index; PostgreSQL-backed HTTP reconciliation remains follow-up coverage. The concurrency test uses independent managers in one process rather than separate OS processes.

Follow-up plan and limits

The committed review report defines four follow-up phases:

  1. Recoverable workspace materialization, fresh-instance checkpoint behavior, storage scrub and interrupted-write recovery.
  2. Incremental metadata persistence, consistent database snapshots, and multi-instance conflict/reload tests.
  3. Streaming transfers with measured memory budgets, explicit state-root semantics, and canonical hash identifiers.
  4. Cross-platform/MSRV/API-contract coverage, realistic restore drills, and workload benchmarks.

Additional restore hardening remains useful: verify archive checksums before extraction, make SQL restore errors unambiguously fail the operation, and seed/verify non-empty datasets in restore tests.

Deduplication now spends sequential read bandwidth to verify content. Overall transfers remain buffered. Missing-chunk reconciliation is a presence check, not a complete integrity scrub. Normal return paths clean private staging files; cancellation or process termination can still leave orphans. This PR does not implement multi-file workspace transactions or a complete power-loss durability model.

After #12/#13 integration, the PR target and final integrated checks should be reconfirmed, especially if upstream changes are squash-merged. No automatic merge is enabled.

Checklist

  • Focused stacked branch; existing PRs and main preserved
  • Review report and prioritized improvement plan committed
  • Windows atomic replacement regression addressed
  • FIFO/special-file existence/read and hidden-storage-error follow-ups addressed
  • 141 workspace tests passed
  • Windows CAS repair tests passed on windows-latest
  • Runtime, Compose, and backup/restore smoke passed
  • Remaining limits documented

@aoruLola
aoruLola marked this pull request as ready for review September 20, 2026 01:58
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 20, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-20T02:36:11.937359Z 4b14a66 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@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: e309d43ddc

ℹ️ 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 crates/server/src/core/storage/atomic.rs Outdated

@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: 77060e9af6

ℹ️ 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 crates/server/src/core/storage/atomic.rs Outdated
Comment thread crates/server/src/api/blobs.rs Outdated

@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: a380603d76

ℹ️ 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 crates/server/src/api/blobs.rs

Copy link
Copy Markdown
Collaborator Author

@codex review

Please review the current head a380603, including the Windows CAS replacement follow-up and the two subsequent storage-safety fixes.

Copy link
Copy Markdown
Collaborator Author

@codex review

Please re-review the current final head 4b14a66, including the centralized special-CAS-entry handling added after the previous review.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 4b14a6605a

ℹ️ 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".

@aoruLola
aoruLola merged commit c7607bb into fix/security-hardening-batch Sep 20, 2026
8 checks passed
@aoruLola
aoruLola deleted the fix/cas-integrity-review-20260920 branch September 20, 2026 02:41
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