Scope integration document-search to the integration's workspace - #162
Merged
Conversation
…rkspace GHSA-fwgr-c6wh-xxff. POST /api/integrations/actions/document-search fanned out across every workspace the linked user belonged to, while document-read looks only in integration.workspaceId and 404s anything else. Search therefore returned path, title, status and a body snippet for documents the same caller could not open. The in-workspace permission filter was never the problem -- searchDocuments already resolves the linked user's role and drops documents they cannot read. The divergence was purely the workspace scope, so search now uses integration.workspaceId, exactly like read. This narrows the cross-workspace fan-out added in #159's sync of the integration search work. The equivalent MCP tools are left alone: there pageden_read_document also spans the user's workspaces, so read and search already agree and no document is reachable by search but not by read. Regression tests cover both halves of the advisory: a document in another workspace that read 404s never appears in search, and a document the linked user lacks permission on is returned neither as a result nor as a snippet.
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.
Fixes GHSA-fwgr-c6wh-xxff.
Summary
POST /api/integrations/actions/document-searchfanned out across every workspace the linked user belonged to.POST /api/integrations/actions/document-readlooks only inintegration.workspaceIdand 404s anything else. Search therefore returnedpath,title,status,updatedAtand a bodysnippetfor documents the very same caller could not open.Search now searches
integration.workspaceId, exactly like read.What the advisory got slightly differently
I reproduced both claims before changing anything. The in-workspace half did not reproduce:
searchDocumentsalready builds aWorkspaceResolverfor the linked user and drops any document whose resolved role is null, so a private folder is correctly filtered and no snippet is generated. Probe againstmain, one integration bound to workspace A, linked user also a member of workspace B:document-read, private folder in Adocument-read, document in Bdocument-searchops/bravo.md, with snippet"# Bravo zebrafish secret plan"So the divergence was purely the workspace scope, which is what this changes. The advisory's "expected" is satisfied either way: a document read denies is now absent from search, including its path and snippet.
This narrows a recently shipped behavior — please confirm
The cross-workspace fan-out was deliberate. It arrived with the integration search work and had a test asserting it (
document-search: 200 fans out across all user workspaces when user is a member of multiple). That test encoded the vulnerable behavior, so it is rewritten to assert the scoped behavior instead.If searching a user's other workspaces through one integration is a product requirement, the safe shape is to bound the fan-out by something the integration is actually authorized for rather than by the user's raw membership list, and to widen
document-readto match. That is a design change, not a security fix, so I did not attempt it here. Say the word and I will spec it.The MCP tools are intentionally untouched
pageden_search_documentsfans out the same way, but so doespageden_read_document— both span the linked user's workspaces, so on that surface read and search already agree and nothing is searchable-but-unreadable. Narrowing MCP would be the same product decision as above. Flagging it because the exposure shape is similar even though the read/search divergence is not there.Numbers
Verification that the tests catch the bug
With the route change reverted:
Test plan
pnpm -r typecheckpnpm -r lintpnpm --filter @pageden/server testpnpm --filter @pageden/server test:integrationFollow-ups not in this PR
allowedFolders; only the workspace-level branches do. Consistent between the two, so not a divergence, but worth deciding whether a linked user's own permissions should be bounded by the integration's folder scope.searchDocumentswithuserId: "", which resolves to no membership and therefore returns nothing. Looks like a functional dead end rather than a leak — separate from this advisory.