Repository navigation
basou: Have the event readers check the session entry themselves, and refuse it with a typed error - #356
Merged
Conversation
… refuse it with a typed error replayEvents and readAllEvents took a session directory and relied on their caller to have checked it; every caller passed an id from a listing, resolveSessionId, assertSessionDirSafe or inspectSessionEntry. They now take the store's paths and a session id, and lstat the entry before reading: a symlink or a file is refused with SessionNotADirectoryError, and a name that is not a session id with "Invalid session id". They do not check .basou/sessions itself, so a federated mirror whose store is a symlink is still read. SessionNotADirectoryError carries the session id and the kind of entry, with the message assertSessionDirSafe, resolveSessionId and basou view's 404 gave. The resolver takes the kind from the listing, which now also returns which of its entries that are not directories are symlinks. The SDK maps the refusal to null by its class instead of matching its message, and its tests now cover readEvents and streamEvents on such an entry.
…at failure, and correct two descriptions The reader tests now refuse a dangling symlink and one to an empty directory, where a reader that looked for events.jsonl first would yield nothing instead, and check that a failed lstat of the entry gives "Failed to read events.jsonl", without a path, with EACCES as its cause. The listing test checks that symlinks is ascending by name, not by creation. loadEntriesFromRoot's doc no longer says a listed session is never checked again: a running session's events go through replayEvents, which checks the entry. The CHANGELOG no longer says the event readers threw a plain Error with the same message before; they followed a symlink and gave another message for a file.
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.
What
replayEventsandreadAllEventsin@basou/corenow take the store's paths and a session id instead of a session directory, and check the entry at the session's name, without following it, before reading:.basou/sessions/<id>SessionNotADirectoryError, kindsymlinkFailed to read events.jsonl(ENOTDIR)SessionNotADirectoryError, kindnot_a_directoryInvalid session idA failed lstat of the entry gives
Failed to read events.jsonl, without a path, with the native error as its cause, as the readers' other I/O failures do. The readers do not check.basou/sessionsitself, so a federated mirror whose store is a symlink is still read; the local store is checked by the listing or resolver that produced the id.SessionNotADirectoryErroris new and exported from@basou/core, withsessionIdandkind.assertSessionDirSafe,resolveSessionIdand the readers throw it, andbasou view's 404 takes its message, which is unchanged:Session <id> is not a directory; a symlink or a file there is not followed. The resolver takes the kind from the listing, soenumerateSessionEntriesalso returnssymlinks, the entries ofnotDirectoriesthat are symlinks.@basou/sdkmaps the refusal tonullby the error's class instead of matching its message with a regular expression.All 19 callers move to the new signature; the federated orientation passes each entry's own store.
Why
The readers relied on a convention: every caller had to pass an id from a listing or a check first. Every caller followed it, but nothing enforced it, and a new caller that forgot would read through a symlink. The refusal's message was also built in three places, and the SDK depended on it matching a regular expression.
User-visible behaviour
None in the CLI or the SDK, since every caller already passed a checked id. The readers' signature in
@basou/corechanges; core is not a semver-guaranteed API.Docs
Verification
pnpm -r build,pnpm -r typecheck,pnpm lint(the same 5 warnings and 1 info asmain),pnpm lint:lang,pnpm lint:paths, andpnpm -r run test(core 2756 passed and 1 skipped, sdk 66, cli 1538): all exit 0.mainand from this branch on the same fixtures (real sessions, a symlink to a copy of one, a file, a dangling symlink, a symlink to a file, pending approvals) across 88 invocations, and found no difference other than relative times and ports. The SDK across 12 inputs, andorientover a federated mirror whose store is a symlink, showed none either.events.jsonl, rethrowing the native lstat error, putting a path in its message, reversingsymlinks) are caught by the tests the second commit adds.Not in this PR
A symlink or a file that is not a regular file at
events.jsonlorsession.yamlinside a real session directory is still read through. A follow-up checks each file before reading it.