fix: reject a non-string workspace id at the adapter boundary - #343
Open
ProfSynapse wants to merge 2 commits into
Open
fix: reject a non-string workspace id at the adapter boundary#343ProfSynapse wants to merge 2 commits into
ProfSynapse wants to merge 2 commits into
Conversation
Observed during triage: passing an object as the workspace id to
`createSession` created `Nexus/data/workspaces/ws_[object Object]/` on disk,
with a shard inside it. Every workspace-scoped write derives its stream from
`workspaces/ws_${workspaceId}.jsonl`, and template interpolation stringifies
anything it is handed — so a bad id becomes a real, permanent stream whose
events belong to no workspace that exists. Nothing validated it: the adapter is
where untyped callers (tool params, plain JS, older call sites) hand data over,
and it had no check at all.
Add `assertValidWorkspaceId` and apply it to the workspace-scoped *write* entry
points, which are the ones that can create a stream:
createSession, updateSession, moveSessionToWorkspace, saveState, addTrace,
createWorkspace (only when an id is supplied — an omitted one is generated),
updateWorkspace, deleteWorkspace
The read entry points were checked and deliberately left alone: getWorkspace,
getWorkspaces, getSessions, getStates, countStates, getTraces and searchTraces
only ever query SQLite by id, so a bad id returns nothing rather than writing
anything. `deleteSession`, `updateState` and `deleteState` take entity ids and
resolve the workspace from the cache, so they cannot mint a stream either.
The guard runs before `ensureInitialized()`, so a bad id fails immediately
instead of riding on a slow start. Eight of the ten new tests fail against the
pre-fix adapter (the other two are positive controls that must pass both ways).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C6aSoCAS5gNoew6n9qv6DJ
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
2 participants
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.
Found during issue triage, not reported by anyone.
The bug
createSessionaccepted an object as a workspace id and created this on disk, with a shard inside it:There was no validation at the adapter boundary, so anything stringifiable became a stream directory.
The fix
assertValidWorkspaceIdruns beforeensureInitialized()on the eight write entry points that can mint aws_*.jsonlstream:createSession,updateSession,moveSessionToWorkspace,saveState,addTrace,createWorkspace(when an id is supplied),updateWorkspace,deleteWorkspace.Ordering matters — before
ensureInitialized(), otherwise a not-initialized error masks the missing guard and the test passes for the wrong reason.Deliberately left alone
getWorkspace,getWorkspaces,getSessions,getStates,countStates,getTraces,searchTraces) — SQLite queries only, they cannot mint a streamdeleteSession,updateState,deleteState) — the id they take is not a workspace idTests
Ten cases, needing a real adapter instance with a ready lifecycle — the delegates are arrow-function class fields, and without a ready lifecycle
ensureInitialized()throws first and hides the gap.8 of 10 fail pre-fix; the two that pass are positive controls proving valid ids still work.
Live: object, empty string,
nulland a number are all rejected with the expected messages, andfind /tmp/test-vault -name "*object*"returns nothing.🤖 Generated with Claude Code
https://claude.ai/code/session_01C6aSoCAS5gNoew6n9qv6DJ
Generated by Claude Code