feat(filestorage): add fail-closed client-side encryption to remote sync

[parthchilwerwar]

Fixes #4562

Describe the changes you have made in this PR -

I added opt-in, fail-closed client-side encryption for remote-sync sessions and memory.

• Added a content-codec boundary without changing the existing four-method ObjectStore protocol.
• Added deterministic authenticated encryption using AES-SIV, with a per-store key derived from an ambient passphrase using Scrypt.
• Kept OPENSRE_REMOTE_SYNC_PASSPHRASE out of config models, config.yml, logs, and user-facing output.
• Added an encrypted verification object for each initialized prefix. It validates the passphrase before any upload can modify remote content.
• Prevented mode crossover: encryption refuses nonempty unverified prefixes, while plaintext mode refuses prefixes already initialized for encryption.
• Made encryption flags strict so a typo cannot silently disable encryption and upload plaintext.
• Preserved ETag-based unchanged-file detection by encrypting identical content deterministically for the same object path.
• Kept --pull-only read-only: an empty encrypted prefix is not initialized until an upload-capable sync.
• Added --encrypt support to CLI and /remote-sync setup, plus status/setup guidance.
• Documented setup, plaintext migration, passphrase rotation, recovery limits, downgrade protection, and provider-visible metadata.

Demo/Screenshot for feature changes and bug fixes -

$ uv run python -m pytest -q tests/filestorage \
    --deselect <five existing Windows CRLF-only assertions>
91 passed, 5 deselected

$ uv run python -m pytest -q \
    tests/interactive_shell/test_remote_sync_cmds.py \
    tests/surfaces/test_remote_sync_surface_contract.py \
    tests/integrations/test_verification_registry.py \
    tests/integrations/test_registry.py
54 passed

$ uv run python -m ruff check config core gateway integrations platform surfaces tools tests/
All checks passed!

$ uv run python -m ruff format --check config core gateway integrations platform surfaces tools tests/
3075 files already formatted

$ uv run python -m mypy config core gateway integrations platform surfaces tools
Success: no issues found in 1550 source files

The five deselected filestorage assertions are the repository's existing Windows CRLF fixture failures; the same tests run normally on Linux CI. The final corrective commit passed CI, CodeQL, synthetic deterministic tests, and the interactive-shell workflow.

---

Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

• No, I wrote all the code myself
• Yes, I used AI assistance (continue below)

If you used AI assistance:

• I have reviewed every single line of the AI-generated code
• I can explain the purpose and logic of each function/component I added
• I have tested edge cases and understand how the code handles them
• I have modified the AI output to follow this project's coding standards and conventions

Explain your implementation approach:

The provider abstraction already has the right responsibility: moving opaque bytes. I kept it unchanged and inserted a codec at the local/object-store boundary. Plaintext configurations use an identity codec, while encrypted configurations derive one in-memory AES-SIV key from the ambient passphrase and store identity.

I considered randomized authenticated encryption, but it would produce different bytes on every run and defeat the engine's ETag comparison, causing unchanged files to be uploaded repeatedly. AES-SIV provides authenticated deterministic encryption when used without a nonce. The logical object path is included inside the authenticated plaintext envelope, so moving ciphertext to another path fails closed. The versioned outer header leaves room for a future format migration.

Ciphertext alone cannot prove that the current passphrase is correct without reading it. Conflict resolution can legitimately avoid downloading an older remote file, so relying only on pull-time decryption would allow a newer local file to overwrite remote data under a wrong key. To close that gap, the first encrypted upload writes a small authenticated verification object. Every later sync verifies it before uploads. The same marker prevents plaintext mode from writing into an encrypted prefix. A nonempty prefix without a marker is rejected in encryption mode, which forces migration to a new prefix rather than mixing formats.

The passphrase is ambient rather than a config field; only the opt-in boolean is persisted. Boolean parsing is strict because treating an unknown value as false would be an unsafe downgrade. Existing plaintext prefixes migrate by pulling first and then pushing to a new prefix. Rotation uses another new prefix so the old data remains recoverable until the replacement is verified.

---

Checklist before requesting a review

• I have added proper PR title and linked to the issue
• I have performed a self-review of my code
• I can explain the purpose of every function, class, and logic block I added
• I understand why my changes work and have tested them thoroughly
• I have considered potential edge cases and how my code handles them
• If it is a core feature, I have added thorough tests
• My code follows the project's style guidelines and conventions

[github-actions]

Greptile code review

This repo uses Greptile for automated review. Before merge, aim for Confidence Score: 5/5 with zero unresolved review threads — see CONTRIBUTING.md.

Run a review — add a PR comment with:

@greptile review

Give it ~5-10 minutes (sometimes longer) for results, then fix feedback and re-trigger until you reach Confidence Score: 5/5.

Optional: automate with the greploop skill.

[Copilot]

Pull request overview

Adds opt-in client-side encryption for remote-sync (sessions/memory) by introducing a local↔object-store content-codec boundary and wiring an encrypted codec (AES-SIV with per-store Scrypt-derived key) through the shared filestorage operations, surfaces, and docs—while keeping secrets (passphrase) ambient and out of persisted config.

Changes:

• Introduce a ContentCodec boundary and implement deterministic authenticated encryption (EncryptedContentCodec) plus codec selection from RemoteSyncConfig.
• Thread codec usage through the sync engine and shared run_remote_sync, and expose --encrypt across CLI and /remote-sync setup with new env constants.
• Add/extend tests and documentation for encryption setup, behavior, and constraints.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tools/interactive_shell/shared/slash_catalog.py	Updates /remote-sync planner/catalog description to mention --encrypt.
surfaces/interactive_shell/command_registry/remote_sync_cmds.py	Adds --encrypt flag handling in /remote-sync setup and shows encryption status.
surfaces/cli/commands/remote_sync.py	Adds --encrypt/--no-encrypt option to CLI setup and persists it.
platform/filestorage/content_codec.py	Introduces ContentCodec protocol + plaintext codec implementation.
platform/filestorage/encryption.py	Implements AES-SIV envelope encryption and key derivation; selects codec from config + ambient passphrase.
platform/filestorage/engine.py	Plumbs codec into push/pull/run_sync and preserves ETag-based change detection on encoded bytes.
platform/filestorage/operations.py	Applies configured codec in the shared remote-sync entrypoint.
platform/filestorage/config.py	Adds persisted/overridable encryption flag without reading config unnecessarily.
platform/filestorage/setup.py	Persists encryption boolean but not the passphrase.
platform/filestorage/messages.py	Updates help/status/setup messaging to include encryption env vars and encryption-on guidance.
platform/filestorage/errors.py	Adds RemoteSyncEncryptionError for fail-closed decryption/envelope problems.
platform/filestorage/init.py	Re-exports new encryption error type.
config/constants/filestorage.py	Adds env var constants for encryption enable + passphrase.
config/constants/init.py	Re-exports the new filestorage env var constants.
docs/configuration/remote-sync.mdx	Documents encryption setup, migration/rotation workflow, and threat model notes.
.env.example	Adds example env vars for encryption.
tests/filestorage/test_encryption.py	New tests for codec determinism, failure modes, and engine integration.
tests/filestorage/test_remote_sync.py	Ensures default (unencrypted) sync keeps provider bytes unchanged; isolates env leakage.
tests/filestorage/test_remote_sync_setup.py	Verifies encryption flag persistence and passphrase non-persistence; env can enable encryption.
tests/filestorage/test_service_and_registry.py	Asserts status messaging indicates encryption without exposing keys.
tests/interactive_shell/test_remote_sync_cmds.py	Verifies /remote-sync setup --encrypt persists encryption and is described in metadata.
tests/surfaces/test_remote_sync_surface_contract.py	Ensures top-level CLI setup supports --encrypt and persists config.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (2)

platform/filestorage/encryption.py:66

• AESSIV.decrypt()’s documented API expects associated_data to be a (possibly empty) list of bytes. Passing None is outside that contract and may break depending on the cryptography version; use an empty list when you have no AAD.

        ciphertext = data[len(_ENVELOPE_HEADER) :]
        try:
            payload = self._cipher.decrypt(ciphertext, None)
        except InvalidTag as exc:
            raise RemoteSyncEncryptionError(

platform/filestorage/encryption.py:53

• AESSIV.encrypt()’s documented API expects associated_data to be a (possibly empty) list of bytes. Passing None is outside that contract and may break depending on the cryptography version; use an empty list when you have no AAD.

This issue also appears on line 62 of the same file.

    def encode(self, key: str, data: bytes) -> bytes:
        """Encrypt and authenticate ``data`` for exactly ``key``."""
        key_bytes = key.encode("utf-8")
        payload = len(key_bytes).to_bytes(_KEY_LENGTH_BYTES, "big") + key_bytes + data
        ciphertext = self._cipher.encrypt(payload, None)
        return _ENVELOPE_HEADER + ciphertext

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

platform/filestorage/encryption.py:67

• AESSIV.decrypt also requires associated_data as a list of bytes; passing None will raise TypeError and prevent any encrypted pull/verification. Use an empty list when no associated data is needed.

            payload = self._cipher.decrypt(ciphertext, None)

[greptile-apps]

Greptile Summary

Adds opt-in deterministic client-side encryption for remote sync.

• Introduces AES-SIV content encoding, passphrase-derived keys, and encrypted prefix verification.
• Enforces encrypted/plaintext mode separation and integrates encryption into shared sync operations.
• Adds CLI and slash-command setup flags, status guidance, documentation, and encryption tests.

Confidence Score: 4/5

The setup downgrade behavior should be fixed before merging because an omitted encryption flag can disable the configured confidentiality boundary.

Both setup surfaces persist false when --encrypt is absent, so routine reconfiguration can stop an initialized encrypted sync or permit plaintext uploads to a new empty prefix; marker initialization also leaves a minor partial side effect when later preflight fails.

Files Needing Attention: surfaces/cli/commands/remote_sync.py, surfaces/interactive_shell/command_registry/remote_sync_cmds.py, platform/filestorage/encryption.py

Security Review

Setup currently interprets omission of --encrypt as disabling encryption, which can unexpectedly stop an existing encrypted sync or send content in plaintext to a newly selected empty prefix.

Important Files Changed

Filename	Overview
platform/filestorage/encryption.py	Adds key derivation, deterministic authenticated encoding, and prefix verification; marker initialization can precede a later local preflight failure.
platform/filestorage/engine.py	Applies codecs at upload/download and ETag comparison boundaries while preserving atomic local writes.
platform/filestorage/operations.py	Wires codec and prefix preparation into all shared sync surfaces, with initialization occurring before engine preflight.
surfaces/cli/commands/remote_sync.py	Adds the encryption setup option, but omission is persisted as an explicit plaintext setting.
surfaces/interactive_shell/command_registry/remote_sync_cmds.py	Adds slash-command encryption setup and status output, but also converts an omitted flag into encryption=false.

Sequence Diagram

sequenceDiagram
    participant User
    participant Setup
    participant Config
    participant Sync
    participant Store
    User->>Setup: setup [--encrypt]
    Setup->>Config: persist encryption mode
    User->>Sync: remote-sync sync
    Sync->>Store: list prefix
    alt encrypted empty prefix
        Sync->>Store: write verification marker
        Sync->>Store: upload AES-SIV ciphertext
    else initialized encrypted prefix
        Sync->>Store: read and verify marker
        Sync->>Store: pull/push encrypted objects
    else plaintext mode
        Sync->>Store: reject marker or sync plaintext
    end

Loading

_{Reviews (1): Last reviewed commit: "fix(filestorage): fail closed before enc..." | Re-trigger Greptile}

[greptile-apps]

Omitted flag disables encryption

When setup is rerun without --encrypt, the option resolves to false and that value is persisted, causing the next sync to fail against the existing encrypted prefix or upload plaintext when the user selected a new empty prefix. The slash setup path has the same behavior because it also derives encryption solely from the presence of --encrypt.

How this was verified: Both setup paths persist false when --encrypt is absent, and plaintext mode accepts an empty unmarked prefix.

[greptile-apps]

Marker precedes upload preflight

The first upload-capable encrypted sync writes its verification marker before push validates every local candidate. When that preflight rejects an unsyncable path, the failed sync still leaves the prefix marked as encrypted, preventing later plaintext use of an otherwise empty prefix.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[parthchilwerwar]

Thanks for reviewing this. I noticed that I had made a mistake in the earlier update, including the formatting failure and a couple of encryption edge cases. I’ve corrected the implementation so setup preserves the existing encryption mode when the flag is omitted, unknown or misspelled flags fail safely, and local files are validated before the encrypted prefix is initialized.

All CI, CodeQL, synthetic, and interactive-shell checks are now passing.

(I'm human sorry for the mistake. It has been fixed in the committed code.)