feat(client)!: surface per-entry updatedAt from the manifest - #5
Open
beardthelion wants to merge 1 commit into
Open
feat(client)!: surface per-entry updatedAt from the manifest#5beardthelion wants to merge 1 commit into
beardthelion wants to merge 1 commit into
Conversation
The server has always recorded when each entry was last written, but nothing outside it could see that: the content response carried ciphertext and checksums, and the hashes view carried checksums alone. Ranking work that wants to prefer a more recently written entry has no way to ask. getData now copies meta.updatedAt into content.entryUpdatedAt in the same pass that fills entryChecksums, after the manifest/blob drift guard, so an entry is never served with a timestamp whose blob is missing. No new value is computed and nothing is read from plaintext; this is metadata the server already authored. pull() sanitizes that field rather than casting it. It is the first thing the client acts on that the crypto does not vouch for: ciphertext is bound to its key by the GCM AAD and checksums are recomputed locally, but a timestamp is whatever the server says, and it is not covered by namespaceChecksum either. Sanitizing walks the decrypted entry keys, keeps an own property per key, and bounds the value by length and parseability. Extra keys, an inherited key, a bare string, an array, non-string values, an oversized string, and an unparseable one all fall away, and the result can never be larger than the namespace. A server that omits the field, and a 404, both yield an empty map. Also closes a pre-existing gap on the same path: a 200 whose body lacks content.entries used to throw a raw TypeError out of Object.entries. It failed closed either way; now it fails with a protocol error that names the fault. What is deliberately NOT checked: whether the server is telling the truth. A well-formed timestamp is accepted whatever it claims, so a hostile server can influence ordering for any future consumer within those shapes. Every guard here is covered by a test proven to fail when that guard is removed. Two of those tests only became load-bearing after the mutation run showed them passing for the wrong reason: an inherited-key test whose payload was rejected by a different check, and a drift test asserted through the client whose own sanitizer masked the server-side invariant it meant to pin. No behavior changes for any current caller: nothing reads the new field yet. BREAKING CHANGE: PullResult gains a required `updatedAt` field. The type is exported from the package entry point, so external code that constructs or mocks a PullResult literal will not compile until it carries the field. It is required rather than optional on purpose, so the compiler finds every stand-in instead of letting one silently diverge from the real client. Values are typed `string | undefined` because a key can legitimately be absent and this project does not enable noUncheckedIndexedAccess. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
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.
The server has always recorded when each entry was last written. Nothing outside it could see that: the content response carried ciphertext and checksums, the hashes view carried checksums alone. This surfaces it.
getDatacopiesmeta.updatedAtintocontent.entryUpdatedAtin the same pass that fillsentryChecksums, and after the manifest/blob drift guard, so an entry is never served with a timestamp whose blob is missing. No new value is computed and nothing is read from plaintext. This is metadata the server already authored.pull()sanitizes the field rather than casting it, because it is the first thing the client acts on that the crypto does not vouch for. Ciphertext is bound to its key by the GCM AAD and checksums are recomputed locally, but a timestamp is whatever the server says, and it is not covered bynamespaceChecksumeither. The sanitizer walks the decrypted entry keys, keeps an own property per key, and bounds the value by length and parseability, so extra keys, an inherited key, a bare string, an array, non-string values, an oversized string and an unparseable one all fall away, and the result can never be larger than the namespace. A server that omits the field and a 404 both yield an empty map.What it deliberately does not check is whether the server is telling the truth. A well-formed timestamp is accepted whatever it claims, so a hostile server can influence ordering for any future consumer within those shapes. That is a real limit and it is why this lands as plumbing rather than as a ranking change.
Also closes a pre-existing gap on the same path: a 200 whose body lacked
content.entriesthrew a rawTypeErrorout ofObject.entries. It failed closed either way, but now it fails with a protocol error naming the fault.Nothing reads the new field yet, so no current caller changes behavior.
Breaking change
PullResultgains a requiredupdatedAtfield, and the type is exported from the package entry point, so external code that constructs or mocks aPullResultliteral will not compile until it carries the field. Required rather than optional on purpose: the compiler then finds every stand-in instead of letting one silently diverge from the real client. Values are typedstring | undefinedbecause a key can legitimately be absent and this project does not enablenoUncheckedIndexedAccess.The squash title needs to keep the
!. Withbump-minor-pre-majorandbump-patch-for-minor-pre-majorboth set, a plainfeat:releases a patch;feat(client)!:releases a minor. I checked that against release-please's own versioning strategy rather than the docs.Testing
70 tests pass, type-check and biome clean. Every guard here is covered by a test proven to fail when that guard is removed. Two of those only became load-bearing after the mutation run showed them passing for the wrong reason: an inherited-key test whose payload was rejected by a different check before it reached the guard it meant to exercise, and a drift test asserted through the client whose own sanitizer masked the server-side invariant it was pinning.
Independent of #4. The two branches touch disjoint files and merge cleanly against each other.