feat: make owner-only state persistence configurable and turn it off for enclaves - #9508
Open
koenvanderveen wants to merge 3 commits into
Open
feat: make owner-only state persistence configurable and turn it off for enclaves#9508koenvanderveen wants to merge 3 commits into
koenvanderveen wants to merge 3 commits into
Conversation
Adds a use_checkpoints flag to the sync engine, defaulting to True so notebook and Colab clients are unaffected. When off, the DO syncer skips checkpoint uploads, rolling-state uploads (one Drive write per event) and the checkpoint reads on cold start, falling through to the existing download-all-events path. Enclaves default to off via SYFT_ENCLAVE_USE_CHECKPOINTS: an enclave boots with fresh_state, so there is never a snapshot to restore and every checkpoint write just spends Drive API calls inside the poll loop. sync(auto_checkpoint=...) is unchanged but subordinate to the config, so no existing sync() call site needed touching.
…t_owner_state The first pass only covered checkpoints and the rolling state, so the DO still wrote its append-only event log to Drive and could still restore from it. That misses the point: an enclave gets an ephemeral keypair each boot, so restoring a previous boot's state does not fit the security model and none of those writes are needed. Renames use_checkpoints to persist_owner_state and extends it to the event log, gated at the single enqueue site in queue_event_for_syftbox. The peer outbox is a separate queue and is unaffected, so peers still receive every change. pull_initial_state now skips the whole owner-state restore rather than falling back to replaying the log; collections stay restored either way since they are peer-facing. Also drops the ValueError guards on the manual checkpoint methods - callers simply do not call them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An enclave gets an ephemeral keypair on every boot, so restoring a previous boot's state does not fit its security model — and it wipes its state anyway under
fresh_state. Every write the DO makes purely so it can rebuild itself later is therefore pure cost, spent on a rate-limited Drive API inside a 1-second poll loop. Until now there was no way to switch that off; the only control was the per-callsync(auto_checkpoint=...)kwarg, and the enclave callssync()with no arguments from ~12 sites.What "owner-only state" means
State read back by nobody but this datasite, existing solely to restore it:
owner_write_events_message_to_syftbox)pull_initial_statePeer-facing state is explicitly not in scope: the outbox, collections and peer files are untouched.
Two things worth knowing, both verified: the owner log and the peer outbox are separate queues drained independently in
process_syftbox_events_queue, so dropping the log cannot affect peer delivery. And the DO's own local changes never reach that log —_queue_events_for_outboxonly fills the outbox — so they were durably recorded only via rolling state → checkpoints. Turning all four off therefore leaves genuinely zero restore state.Changes
persist_owner_stateflag, defaultTrue, so notebook and Colab clients are unaffected. It sits onDatasiteOwnerSyncerConfigandSyftboxManagerConfigand is threaded through every factory, mirroringuse_in_memory_cache.queue_event_for_syftbox, which avoids an unbounded queue rather than draining-and-discarding.pull_initial_stateskips the whole owner-state restore when off, rather than falling through to replaying the log. Collections are still restored either way.sync(auto_checkpoint=...)is unchanged but subordinate, so no existingsync()call site needed touching. The automatic path returnsNone; the manual checkpoint methods are simply not called rather than raising.EnclaveSettings.persist_owner_state(SYFT_ENCLAVE_PERSIST_OWNER_STATE), flowing throughfor_enclaveinto the DO syncer config. Both enclave entrypoints pass and log it.pull_initial_statesplit up. It was ~100 lines and its restore half is exactly what needed skipping, so it is now_restore_owner_stateplus one helper per step. One subtlety preserved: a full checkpoint carrying nolast_event_timestampmust not trigger a download-all, hence_CheckpointRestore.found_checkpointbeing tracked separately from the cursor.Deploy
Because the default is
False, every deploy path is already correct with no template change —just start,start-debug,terraform apply,local-runand theinference-*variants all get it for free. The only deploy edit is addingSYFT_ENCLAVE_PERSIST_OWNER_STATEto both Dockerfiles'allow_env_overridelabels (Confidential Space rejects un-allowlistedtee-env-*overrides) so it can be switched on later. The Justfile / terraformtee-env-keys are deliberately left alone.Test plan
just test-unit: 431 passed. The 2 failures intests/unit/syft_bg/test_init.pyare a pre-existing macOS/private/varsymlink issue, confirmed on a stashed clean tree.just test-unit-rds(79),just test-unit-enclave(73),just test-unit-enclave-model-api(5): all pass.rolling_state.json) while the peer still receives every change; a cold start restores nothing and attempts zero event downloads; the owner state is all present by default; enclave settings default + env override; and a regression test on the**kwpassthrough insyft-rds, which nothing type-checks and where a rename would silently hand an enclave back the state it must not keep.pre-commit runpasses on all changed files.Not run: the
just local-runend-to-end check, which needs docker plus real Drive credentials and writes to a live datasite.