refactor(backups)!: gate restore on graph type, retire the encryption flag - #1102
Merged
Conversation
The `encryption` flag never encrypted anything. `security/encryption.py` had no production callers, so the flag only toggled two policy gates: encrypted backups could not be downloaded, unencrypted ones could not be restored. Since the default was `encryption=false`, the default backup was silently unrestorable, and the stated reason for blocking downloads — that the payload would be undecryptable — did not hold. Restore availability is a property of the graph, so gate it there: - Entity graphs are materialized from the extensions database; restore stays refused, and the guard now fails closed when the graph row is unresolvable instead of falling through. - Shared repositories stay download-only (403, unchanged). - Generic and private repository graphs are restorable, and every completed backup is downloadable. `restore_supported` is added to the backup list response so clients can gate the action without inferring it. `BackupCreateRequest.encryption` and the two response fields are kept, deprecated and inert, so this rides an SDK minor; they are due for removal in the next client major. Objects remain encrypted at rest with S3 SSE-AES256 and are served over TLS through short-lived signed URLs. SECURITY.md claimed application-layer Fernet encryption of backups, which was not the case; corrected to describe what actually protects them. `GRAPH_BACKUP_ENCRYPTION_KEY` is no longer read and is dropped from the bootstrap; the existing secret can be deleted separately. Also fixes the admin backups endpoint, which summed a `size_bytes` attribute that is not a column on `GraphBackup` and raised on any graph with a backup, and adds the first tests for the restore route.
Giving encryption_enabled and allow_export Pydantic defaults dropped them out of the OpenAPI required set, widening the generated client types from boolean to boolean | undefined. The server always sends both, so that is a breaking read for consumers with nothing gained — it would have forced an SDK major rather than a minor. Deprecate them in place instead.
Drops `encryption` from BackupCreateRequest and `encryption_enabled` / `allow_export` from BackupResponse rather than carrying them deprecated. Nothing reads them: the flag never encrypted anything and no longer gates download or restore, and `allow_export` was always the inverse of it. Removing response fields is a compile-break for a typed consumer that references them, so the client regen is a deliberate, coordinated change rather than silent drift. The `encryption_enabled` column stays on GraphBackup. It is the only record of which backups were once marked encrypted, which is worth keeping even though nothing acts on it.
jfrench9
added a commit
that referenced
this pull request
Aug 8, 2026
…on't have `total_gb_hours` was a storage-billing unit. There is no storage billing: `storage_overage_gb` is a column nothing ever writes, the billing config has no storage entries at all, and the frontend's `storage_billing_enabled` / `storage_rate_per_gb_per_day` are dead fields the API never returns. Storage is included in the tier. So the endpoint published a billing figure, labelled "for billing", for a mechanism that was removed. This retires the concept rather than the arithmetic. #1095 fixed the computation two days ago — correctly, but on a premise that was already false. A correct number nobody should be reading is worse than an obviously wrong one, because it invites use. BREAKING: `StorageSummary.total_gb_hours` is removed from `GET /v1/graphs/{graph_id}/usage`. Both published clients carry it in their generated models and need a regen; same shape as the backup encryption-field removal in #1102. No frontend reads it (grepped robosystems-app). The time-weighted machinery stays, because it was never really about GB-hours: `avg_storage_gb` is now computed by `_time_weighted_average_gb`, weighting each reading by the span since the previous one. A plain mean would over-count whatever the sensor happened to sample more often, and the snapshot interval lives in another module and has moved before. The span cap survives for the same reason — it bounds how far one stale reading can pull the average after an outage. Kept, deliberately: `GraphUsage.storage_gb` and the `STORAGE_SNAPSHOT` rows. Those are the measurement, not the billing unit, and the capacity work reads them. `storage_overage_gb` also stays for now — dropping a column wants its own migration. Tests: the cadence-independence case is rewritten against the average (same property, still fails a plain mean). The outage test was rewritten too — it had two readings, where the cap changes nothing and the assertion passed either way; it now uses three, so capped (4.2) and uncapped (4.9) differ and it actually tests the cap.
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
1 participant
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.
What
The backup
encryptionflag was inert — it performed no application-layer encryption and only toggled two policy gates: encrypted backups could not be downloaded, unencrypted ones could not be restored. Since it defaulted tofalse, the default backup was silently unrestorable.Restore availability is a property of the graph, so it's gated there now, via one helper shared by the restore route and the list response:
materializeThe entity guard also fails closed when the graph row is unresolvable; previously it fell through.
Also in here
restore_supportedadded toBackupListResponseso clients can gate the action without inferring itencryptionremoved fromBackupCreateRequest;encryption_enabledandallow_exportremoved fromBackupResponse.enckey suffix, anis_encrypted/encryption_enabledmetadata key mismatch, and an inertencryptedform field on the internal graph_api boundaryGRAPH_BACKUP_ENCRYPTION_KEYis no longer read; dropped from config and bootstrap. The existing secret can be deleted separately.SECURITY.mdupdated to describe the protections actually in force for backups (SSE-AES256 at rest, TLS in transit, short-lived signed URLs)size_bytesattribute that is not a column onGraphBackupand raised on any graph with a backupencryption_enabledcolumn stays onGraphBackupas the historical record of which backups were once marked encryptedCoordination
Clients regenerated and published as 1.6.0 (TS #184, Python #177). Frontend in robosystems-app picks up 1.6.0 and reads
restore_supported; it carries a fallback so it behaves correctly against an API that predates this change.Removing response fields is breaking for a consumer that reads them. Verified none do — across
robosystems-app,@robosystems/core,roboledger-app, androboinvestor-app— and released as a minor deliberately rather than carrying dead fields through a deprecation cycle.Tests
just test-all— 12,309 passed, 23 skipped; ruff, format, basedpyright, cf-lint clean. Addstests/routers/graphs/test_restore_backup_op.py, the first coverage the restore route has had.Deploy note
A
backup_graph_jobenqueued before this deploy and executed after will carry anencryptionkey thatBackupGraphConfigno longer accepts. Backup jobs are short-lived, so the window is small — avoid deploying mid-backup.