Repository navigation
Conversation
Signed-off-by: Tim Nunamaker <tim@opendatalabs.xyz> Assisted-by: AI Signed-off-by: Tim Nunamaker <tim@opendatalabs.xyz>
Signed-off-by: Tim Nunamaker <tim@opendatalabs.xyz> Assisted-by: AI
A completed newer Oura import could be rejected when its current records omitted an older ID. The importer required historical coverage, and the shared reader separately treated every retained tombstone as incomplete. Allow a strictly newer complete import to replace the current dataset. The shared reader accepts omitted IDs only when all three Oura streams have current, coherent completion receipts newer than the legacy heads. Keep same-time parity checks, stale-import rejection and the existing account, method, generation, head and write fences. Exercise smaller, different-key and empty sets through the signed import route, including incomplete controls, restart and unchanged historical files. Also cover mixed-run receipts and recovery after an incomplete replacement. Installed Desktop and phone upgrade proof remains pending. Signed-off-by: Tim Nunamaker <tim@opendatalabs.xyz> Assisted-by: AI
Signed-off-by: Tim Nunamaker <tim@opendatalabs.xyz> Assisted-by: AI
An older completed import could hide a newer saved file because receipt presence alone enabled whole-dataset replacement. Require every input stream's completion time to be newer than the saved head. Older and equal-time imports retain the existing body-comparison policy. Without a reader for the old body, keep the file before takeover or refuse the read instead of recording an unscoped takeover. Invalidate witnesses created under the old rule. Tests preserve newer files against old imports, reject stale account and generation receipts, and verify that fresh completion can still replace data. Declaration changes also invalidate the persisted replacement cursor. Assisted-by: AI Signed-off-by: Tim Nunamaker <tim@opendatalabs.xyz>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tim@opendatalabs.xyz>
An Oura import could replace the current history with a window that starts later, so the earlier days stop appearing in what owners and the apps they authorize can read. The window is the start day that the import's selection declares for the sleep, readiness and activity streams (`time_range.since`). Example: the stored history starts on 2026-01-01, and an import with a window starting on 2026-10-01 completes with no records. Before this change, the import was accepted, and each stream's head (its newest stored file) was replaced by an empty batch. The older files, written in the legacy file format, stay on disk but are no longer what reads return. The importer now compares the window with the earliest day of the current history before it writes anything. It reads that day with `readOuraCollectionRequirements`, the function the requirements endpoint uses to tell clients where collection must start, so the check and the client guidance cannot disagree. The check runs under the existing Oura import lock (the scope lock that serializes Oura imports), and it reads the history after taking that lock. A window prepared earlier therefore cannot authorize truncating history that arrived after preparation. A later window is refused with `selection_narrower_than_history` before any canonical write, commit callback (the Node hook that writes the canonical record store), receipt (the per-stream result returned to the caller), or file change. Unchanged: an unrestricted selection (no start day, so all history) is never narrower. Absent or empty history has no earlier day to lose, so any window is accepted. A window that starts on or before the earliest day is accepted, so complete replacements that are smaller, empty, or have different record IDs still work. Native publications carry no selection, so the importer cannot see the start day that the native collector (a signed collector artifact outside this repository) used. That start is unproven, so a native batch is refused with `native_start_unproven` whenever the current history contains any dated row. A native batch that includes a selection is checked like any other batch. Until the native publication sends its selection, native batches are refused over any existing history. The native collector's behavior is not verified here. Not covered: the Node route reads its retained rows (current records, including deleted IDs, kept by the Node store) before taking the lock and passes that snapshot in. Those rows are not re-read under the lock, so a change to them that the heads and the canonical view do not show is not seen by this check. Tests cover these scenarios. Lite (packages/lite/src/oura-import.test.ts): an October-start window over January history is refused whether the batch is complete with records or complete and empty, and the heads, the stored file count per stream, what owner and builder reads return, the commit callback, sync notifications and receipts are unchanged. A window prepared before January history arrived is refused after that history is imported. A window starting exactly on the earliest day, a window over no history, and unrestricted replacements still succeed. Node (packages/server/src/pdpp-oura-legacy-first-journey.test.ts): an empty October-start POST over legacy-format history from 2026-09-01 returns 409 and leaves what owner and builder reads return unchanged for all three streams. On the parent commit, five of the new Lite tests failed, and the Node POST returned 200 with receipts. Not run: the native journey tests, which run only when the environment variable F2_NATIVE_PUBLICATION_ORACLE (the path of the native publication test binary) is set, and it was unset here. The root test suite was not run. Assisted-by: AI Signed-off-by: Tim Nunamaker <tim@opendatalabs.xyz>
The previous commit refused native batches without a selection whenever the current history had dated rows. The desktop app's native publication (the batch its collector posts) sends no selection, so that refusal blocked native publication entirely. This change keeps the narrower-history guard and makes the native START explicit instead of exempting native batches. Every Oura batch, native or browser, must now carry the START selection it was collected with (the window the collector was started with), in `selection`. The importer validates it with the same takeover validator and compares its common `time_range.since` with the required boundary under the import lock. A missing or invalid selection is refused with `selection_not_eligible`. The artifact digest and binding identify the collector that ran. They do not prove the window, so they no longer stand in for it. The Node route no longer captures retained history before the lock. It passes a reader that the importer calls under the import lock, so the boundary check sees the current rows. The epoch fence is unchanged: the epoch is still captured before the lock and compared inside the write transaction. A commit that changes the epoch after capture is refused with `canonical_history_changed`. Removed from the previous commit: the `artifactBoundNativeCompletion` option and the `native_start_unproven` reason. Tests: Lite covers a batch without a selection, with and without January history, and a retained-history read that happens after an earlier batch commits under the lock. Node adds a native batch that forwards a valid selection at the required boundary; it is accepted and replaces the canonical data. A narrower since-only native batch and an unselected native batch are refused, and owner and builder reads are unchanged afterwards. The native oracle tests stay skipped without F2_NATIVE_PUBLICATION_ORACLE. Assisted-by: AI Signed-off-by: Tim Nunamaker <tim@opendatalabs.xyz>
An owner token limited to one connected account could let a client read another account of the same source. The same token could also list, relabel or delete that account's connection. An owner token is a bearer credential (a token sent in an HTTP Authorization header) that the server issues to its owner, the person whose data the server holds. It can carry a list of instances: the connected accounts of one source (a data provider's integration, such as Spotify). The connection routes call these accounts connections. The token resolver (the code that looks up a token) read that list and then dropped it. The consent routes (the HTTP endpoints for authorize, review, approve and deny) and the connection routes checked only the source, so the list never reached them. Consent is the owner approving a request from a client (an app) for access. The owner first sees a review (a summary of what would be granted). The approved result is a grant, a stored permission that names data streams (named collections of records) and instances. The token record the server looks up for each request now keeps the list, and every consent lookup is narrowed to it. A request, review choice or approval that names an account outside the list gets 403 (forbidden) before the server works out what the grant would contain. An approval also checks the grant it is about to store against the list, before writing anything. A review prepared with a source-wide token (one with no list) and approved with a narrower token fails either way. Naming the outside account returns 403. Omitting it returns 409 (conflict), because the recomputed review no longer matches the review the owner saw. Connection listing is filtered to the list. Creating, updating and deleting are refused before any connection lookup or write, so an id outside the list gets the same 403 whether or not that connection exists. Two behavior changes follow. A token with a list now resolves an omitted account to its own account; before, it got a choice that named accounts outside the list. An empty list now covers no accounts; before, the list was dropped, so an empty list acted as the whole source. An absent list still means the whole source. Tests run over real HTTP and SQLite. A journey test (an end-to-end test of one whole flow) mints an owner token through the real route, then asks for account B at review and at approval. It also approves with a digest (a fingerprint of a review) that a source-wide token produced for B, using an A-only token (a token whose list holds only account A). After the refused attempts, the raw tables hold no grant, no one-time authorization code (a code a client exchanges for a token) and no client token. A direct read of B with the same A-only token still returns 404. Consent for account A alone still issues a grant over A only. Connection tests show that creating a new account, relabeling account B and deleting account B with an A-only token return 403 and change nothing. Tests whose scenario spans the whole source now use a source-wide token. A malformed streams field still gets 400 (a test pins this). Not verified: I did not trace every caller of the functions that delete connections. The record routes (which read stored data) and the stored-file routes (which serve stored files) already read the list; I did not change them, and I read their filters, not every path. The grant recheck has no test. Current resolution filters every account through the list before it can grant one (I read resolveInstances, the function that picks accounts for each stream), so no test can reach the recheck. The same change applies unchanged to bf6e0df, the pinned release commit, which lacks the fix in 5959c3a that parses consent query keys (such as instance[name]) safely. I ran the tests on that tree and did not assess its parser. Not run: the full server suite and the end-to-end tests. Assisted-by: AI Signed-off-by: Tim Nunamaker <tim@opendatalabs.xyz>
Assisted-by: AI Signed-off-by: Tim Nunamaker <tim@opendatalabs.xyz>
A scope delete erased the whole local scope after its remote blob deletes awaited, so an owner write accepted during that await was erased with the covered data and its import operation was retired. - Erase only the rows captured before the remote await. Each erase carries a VersionFence (row id and path), checked at the erase boundary: inside the owner-import immediate transaction on the server, under the scope lock in Lite. Callers that pass no fence keep the unconditional erase. - Drop the derived first-seen sidecar after the fenced erases, under the scope lock, so a stale ledger cannot survive a re-add. - Persist the successor marker before dropping its predecessor (range advance and unknown-marker expansion), so a failed write cannot drop an owed key. Limits: the Lite fence is single-runtime; the scope lock is process-local. Raw port writes that reuse a live path are not guarded, and multi-tab use is not covered. Assisted-by: AI Signed-off-by: Tim Nunamaker <tim@opendatalabs.xyz>
Member
|
Review (Kahtaf's agent-assisted review) — merge after fixes. No data-loss/auth-bypass found; legacy files never deleted by replacement.
Also: CI never ran tests on this PR ( |
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.
A successful fresh sync could leave readers on the old connector file or fail when the new dataset omitted records that existed before.
Persist
replacement_cursoronly when every input stream has current completion proof newer than the saved head. Oura also requires all three streams to complete at one collection time. Older or equal-time imports retain the existing parity checks. A missing old-body reader cannot grant an unscoped takeover. Account, method, generation, head and writer fences remain. Before takeover, an incomplete projection keeps the legacy file after rechecking ownership.Validation exercises smaller, different-key and empty datasets through the signed import API, checks exact owner and builder reads across restart, and confirms historical files remain unchanged. Negative controls reject missing or partial completion, wrong record counts and mixed-run receipts. SQLite tests also cover receipt identity, confirmation races, migration from version 14 and retained history.
The index schema advances from 14 to 15, so older runtimes reject upgraded indexes. Installed Desktop and phone upgrades and a supported downgrade path remain unverified.
Stacks on #459.
Assisted-by: AI