Repository navigation
Let admins list and inspect workspaces, audited - #1026
Maximo-Guk wants to merge 29 commits into
Conversation
|
LGTM! |
2903b4d to
7c7af62
Compare
|
LGTM! |
7c7af62 to
498f740
Compare
Preview:
|
|
LGTM! |
498f740 to
4269aa5
Compare
| ...(refused ? {refused} : {}), | ||
| }; | ||
| try { | ||
| await impl.adminAudits.getByName("").append(record, policy.retentionDays); |
There was a problem hiding this comment.
[P1] Prevent stale KV retention from pruning the audit log
#call() builds this policy from readAdminConfig()'s eventually consistent KV mirror, but append() immediately calls #prune() with the supplied retention. After an admin raises retention from 90 to 365 days, a workspace still seeing 90 can permanently delete records that cross the 90-day boundary during propagation, even though they are now within the committed retention. I reproduced this with the actual audit-store class and in-memory SQLite: a record retained when the setting changed was deleted by an inspection append 20 seconds later. Keep destructive pruning on the existing authoritative-config paths (listAdminAudit() / getAdminAuditStats()), or otherwise prevent inspection appends from pruning with stale retention. The package guide identifies the source of truth: “The AdminSettings durable object owns the authoritative AdminConfig” (packages/workshop-backend/AGENTS.md).
|
Review: 1 finding. Submitted one actionable inline comment, verified with an in-memory reproduction. |
4269aa5 to
2105261
Compare
|
LGTM! |
2105261 to
d085e19
Compare
|
LGTM! |
d085e19 to
40561b7
Compare
|
LGTM! |
40561b7 to
aadc76a
Compare
Clear filters reset the filters, but text typed in the admin or owner field and not applied stayed, and Enter then applied it. The fields now drop it whenever every filter is cleared. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TypedFilterField hid what was typed when its filter changed from elsewhere, and brought it back when the filter came back to the value it was typed over. It now drops it as soon as the filter, or a clear of every filter, changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A username has no length limit, and workspaceOwner was kept whole, so a page of records naming a very long owner could pass the RPC size limit and leave the Audit tab unable to load it. The store now cuts the owner as it cuts the title, and the owner filter cuts its query the same way, so a long owner's whole id still finds its records. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cutting an owner's id like a title made two usernames sharing their first 255 characters one owner in the log, and the owner filter mixed their records. An id over MAX_ADMIN_AUDIT_ARG_LENGTH is now kept as its start, "…#" and 20 hex digits of the whole id's SHA-256 (auditOwnerId()), still bounded, and the owner filter applies the same, taking the whole id or the kept one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The activity log gains a Target user column: for a call into a workspace, the workspace's owner, the user the call reached, which filters the log by that user; a setting change reaches every user and shows none. The owner filter is named Target user to match, and offers the target users the shown page names. The column that held the workspace or the change is now Details, and the hidden heading of the row toggles' column "Expand", so that no two columns share a name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The log's row is now time, admin, action, sensitivity and target user. The workspace or the changes, and how the call came, move into the row's details, where Via joins them (the owner line there went, the target user column holding it). The table sizes its columns to their content and gives the action what is left, so on a narrow card the action is cut (whole in its tooltip) rather than a name, and the log no longer needs a horizontal scrollbar. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
getAdminAuditStats() no longer ranks workspaces (topWorkspaces): the Audit tab stops showing them,
and listAdminAudit({ workspace }) lists every call into one, with its content reads through
`exposed`. The store drops the query that ranked and titled them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The card goes with the statistic behind it. A workspace is still chosen as a filter from a record's details in the log, named by the title the log's records carry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
It ordered the top workspaces as well until they were dropped; its name and fixtures now say only what it checks, the top admins across both kinds of record. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#1040 landed resizable, remembered log columns, striped rows, a return to the newest page after a settings change, focus kept on the filter field after a name is chosen in a row, and Escape abandoning a half-typed filter. They now fit this log's columns: Time, Admin, Sensitivity and Target user resize, and Action, rather than Changes, takes the width left over, so a narrow log cuts the action while a name too long for its column wraps. Escape and the kept focus reach the target user field as well. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ll's badges Workspace metadata and Refused wrapped onto two lines in the Sensitivity column's default width. It starts at 240 px now, with Admin and Target user at 140, so the log still fits the Audit tab at a 1440 px window. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…bjects alone The git cache gains readers that answer from the objects it already stores and never pull: localPathEntryAtCommit, readLocalFileAtCommitIfExists and changedLocalFilePathsBetween, with readCommitLog taking stopAtMissing so a walk ends where the stored history does. The tree diff takes its tree reader as an argument, so the local and the pulling diff share one walk. In the overseer, buildChatContent, getCurrentChatContent and the worktree edit-base seeding take the reader of a file's base text, which defaults to readFileAtCommit, and worktreeTouchedPaths counts live rows beside the stored messages, so a caller that must not pull can fold a chat's code too. Co-Authored-By: Claude Code <noreply@anthropic.com>
A connection's stored record carries createdBy, the author who created it. The session paths (newGatekeeper, newAiModelGatekeeper and newAgentSpawnerGatekeeper) record the calling client's profile, and createExternalResource records the initiator of the creating chat's own turn, leaving the field out when that turn has no record. An ambient capsule is created by no one, so it has no creator. Nothing reads the field yet; it is there so a connection can later be traced to a person. Co-Authored-By: Claude Code <noreply@anthropic.com>
OverseerImpl gains presenceSnapshot(), blueprintSummaries() and shareLinkInfos(), the bodies that addPresenceSubscriber(), listBlueprints() and listShareLinks() held inline, with the two private blueprint helpers moving along. shareLinkInfos() takes the reader of the owner's profile as an argument, so the client interface keeps passing its own whoami() and a caller that holds no client session can supply another. The client methods delegate, and behaviour is unchanged. Co-Authored-By: Claude Code <noreply@anthropic.com>
AdminWorkspaceView gains getChatState(), listChatAttachments() and readChatAttachment(), so an admin can see why a chat looks stuck (its recorded turn, running or paused, its newest entry, the calls queued for its agent and its pending actions) and what was attached to it. Each answer names its fields one by one, since the stored records hold more: user ids, raw call arguments, scratch paths and bytes. Below "full" a queued call's argument summary is emptied and readChatAttachment() is refused as content, while an attachment's name, type, size and content hash stay, as titles do. OverseerImpl.getChatAttachmentData() takes a `local` flag that reads a legacy attachment only from the objects the workspace stores, so the view still never reads through a connection. Co-Authored-By: Claude Code <noreply@anthropic.com>
…ding changes AdminWorkspaceView gains getCommitLog(), listFilesAtCommit(), readFileAtCommit(), listChangedPaths(), listPendingChanges() and readPendingFile(), so an admin can see how a gadget's code got to where it is and what a chat would change before it is accepted. Each read answers only from the objects the workspace stores: a parent that is not stored ends the walk, and a worktree's base is never pulled through its connection. The log's depth is bounded at MAX_ADMIN_COMMIT_LOG_DEPTH. Below "full" the two file reads are refused as content and a commit message is emptied, since it describes its change, while commit ids, authors and path names stay, as file names do. Co-Authored-By: Claude Code <noreply@anthropic.com>
…is present AdminWorkspaceView gains listObservers(), listObserverRequirements(), listShareLinks(), listAutoApprovals() and listBlueprints(), so an admin can see who a workspace is shared with, what sharing it asks of a recipient and which actions its members opted to apply without a manual approval. Each read answers from the workspace's own records and never reaches a connection, and a share link never carries a key that redeems it. Below "full" a blueprint's id is emptied and its screenshot URL left out, since the id is a share link to its code, while titles, descriptions and link notes stay, as a workspace's title does. listGatekeepers() adds each connection's creator, and getDiagnostics() the storage size and the presence roster, which an admin view never joins. Co-Authored-By: Claude Code <noreply@anthropic.com>
The admin inspection suite seeds a workspace with a merged gadget, a chat attachment and a draft chat whose pending changes include a file just over MAX_BUFFERED_BLOB_SIZE, then calls every read AdminWorkspaceView offers and checks both the answer and the audit record it leaves, including the large file read to its end as a stream. It pins what is withheld below "full": attachment, commit and pending file reads are refused as content and recorded with their arguments, commit messages are emptied, and an invalid log depth is rejected with no record. It also checks that a connection names its creator only when a member made it, that admitted and refused observers are both listed, that a share link's key never appears in an answer, and that the presence roster holds the owner and never the admin. Co-Authored-By: Claude Code <noreply@anthropic.com>
…it tab's help docs/admin-access.md lists every AdminWorkspaceView read in the interface's groups, with the caveats for listObservers(), listPendingChanges() and listAutoApprovals(), and says what shows of a commit a connection supplied and who is named as a connection's creator. Its account of a call names the content methods refused below "full", the failed reads that are recorded, the unrecorded log depth and how a `stream` answer is audited, and the levels, exposure table, recorded refusals, error codes and tests follow. docs/observers.md extends the admin exception to attachments, commit messages and connection-supplied repositories, and the Audit tab's help sentence says what "Metadata only" withholds, with a test, so an admin reads the same rule where the level is chosen. Co-Authored-By: Claude Code <noreply@anthropic.com>
…folded listPendingChanges() awaits the chat's folded content, then reads its pins and change rows. The fold settles one microtask before the listing goes on, and an accept's synchronous tail can run in that gap. The listing then held the files from before the accept against the pins the accept had cleared, and reported every file as added, with no base. It reads the chat's code base and message sequence on both sides of the fold and takes the fold again when they moved, as the fold itself does while it builds. Co-Authored-By: Claude Code <noreply@anthropic.com>
docs/observers.md listed the descriptions and fields of observations as restricted data. The view withholds those of actions and hooks below "full" as well, as docs/admin-access.md says, so the two documents name the same three. Co-Authored-By: Claude Code <noreply@anthropic.com>
The comment in AdminWorkspaceViewImpl.listPendingChanges() said the pins it reads are the ones the content was folded from. That does not hold for a cached fold: a step that only commits in a worktree pins it without moving the generation or the revision the cache is keyed by, so the cached content has no entry for that worktree and the listing leaves it out, where a fresh fold lists it with three empty lists. No path or base is reported wrongly. The comment says so; the behaviour is unchanged. Co-Authored-By: Claude Code <noreply@anthropic.com>
The list of restricted data named messages, attachments, files, commit messages and the text of actions, observations and hooks. An inspection also withholds the argument summaries of queued agent calls and blueprint ids below "full". The list names them, and points at the table in docs/admin-access.md that says what each call withholds. Co-Authored-By: Claude Code <noreply@anthropic.com>
d47427a to
27434f0
Compare
There was a problem hiding this comment.
✅ Approved — No findings. The one INFO from the last review (an incomplete restricted-data list in docs/observers.md) is fixed, and nothing changed since then has a defect.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🔒 Security: ✅ LGTM
- Access cap: the cap fails closed. An unrecognised
ADMIN_WORKSPACE_ACCESS_MAXbecomesoff, andsetAdminWorkspaceAccessrefuses a level above the cap. Every call runs at the lower of the level asked for and the current policy. - Content refusal: below
full, the five content reads are refused, and commit messages, queued-call argument summaries, the descriptions of actions, observations and hooks, and blueprint ids are withheld. - Recording order: the audit record is written before the answer leaves the server.
- SQL: audit and directory queries bind every value as a parameter, and filter lengths are capped.
📖 AGENTS.md: ✅ LGTM
The AGENTS.md updates match the change.
📋 Code Quality: ✅ LGTM
- View calls:
AdminWorkspaceViewImpl.#callchecks expiry and admin status before the read and again before it records the call, with no await between the last check and the audit append. A refused call is recorded and then refused. A failed read is recorded asmetadataand rethrown. A call whose record cannot be written fails and returns nothing. - Local-only reads: the new git-cache readers use only stored objects and never pull.
readCommitLog({ stopAtMissing })marks a parent as reached before skipping it, so the walk never visits it twice. - Directory and paging: sync steps run one at a time and re-check the removed flag at each step. Pages fetch
limit + 1rows in id order, so no row is skipped between pages. vitest.config.ts:ADMINSis now an array binding, which matchesstring[] | stringinenv.d.ts.
⚡ Performance: ✅ LGTM
Each admin view call does one config read and one audit append. Indexes cover the new filters, and the retry loop in listPendingChanges is bounded.
📝 Documentation: ✅ LGTM
The list of restricted data at docs/observers.md:118-120 now names the argument summaries of queued agent calls and blueprint ids, so it matches docs/admin-access.md.
Reviewed commit: 27434f04
|
LGTM! |
Eval resultsVerdict: ⚪ Unchanged. No task moved beyond what these runs can tell apart from noise.
Failed checks
|
🔬 Eval runs reviewPerformance
🟡 VERDICT: INCONCLUSIVEThe diff’s audited admin views and directory feeds do not explain the opaque candidate-only GPT failures, while the single comparable Sonnet run provides too little evidence to clear or implicate this PR.
TriageFailure modes
Tool errors
What to do
|
Builds on #1040, now merged. Lets a deployment admin list users and workspaces and inspect any workspace read-only, with every call recorded in the audit log, and shows those calls in the Audit tab with their sensitivity. Supersedes #861.
Kernel (first commit)
AdminApi.listUsers/listWorkspaces, read from the user directoryAdminApi.inspectWorkspace(id, reason, { access, lifetimeSeconds })mints a read-onlyAdminWorkspaceView(describe,listChats,getChat,listActions,listFiles,readFile,listGatekeepers,getDiagnostics) throughOverseerDurableObject.openForAdmin, which is reachable only over the DO binding. The view:docs/observers.md); the audit is the compensating controloff/metadata/full. A view runs at the lower of the level asked for (metadataby default) and the deployment's, which is capped byADMIN_WORKSPACE_ACCESS_MAX(defaultmetadata; anything unrecognised meansoff).AdminAccessRecordafter the server-side read and before anything leaves the server. The record says what the answer exposed (metadataorcontent) and who owns the workspace (workspaceOwner, resolved when the view opens)UI (second commit)
More workspace reads (last nine commits)
getChatState(the recorded turn, queued agent calls, pending actions),listChatAttachments,readChatAttachmentgetCommitLog,listFilesAtCommit,readFileAtCommit,listChangedPaths,listPendingChanges,readPendingFile, at any commit the workspace storeslistObservers,listObserverRequirements,listShareLinks,listAutoApprovals,listBlueprintslistGatekeepersadds who created each connection (recorded from now on, so older ones have none), andgetDiagnosticsthe storage size and who is presentexposedset from the answer, and only stored objects are read, never a connection. Belowfullthe threeread*calls are refused, and commit messages, queued-call argument summaries and blueprint ids are withheld.Screenshots (three admins, real inspections at
full)Before (main): settings changes only.
After: content reads per admin; the log with a content read opened; carol's
getChatcalls, a refused read opened; page 2.