Skip to content

fix(knowledge): enforce trusted workspace scope - #6452

Open
TheodoreSpeaks wants to merge 6 commits into
improvement/v2-endpointsfrom
fix/v2-copilot-knowledge-scope
Open

fix(knowledge): enforce trusted workspace scope#6452
TheodoreSpeaks wants to merge 6 commits into
improvement/v2-endpointsfrom
fix/v2-copilot-knowledge-scope

Conversation

@TheodoreSpeaks

@TheodoreSpeaks TheodoreSpeaks commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • consume the merged delegated-identity foundation and send server-authored workflow/execution scope through shared executor headers for all 14 internal Knowledge HTTP tools
  • resolve document, chunk, tag, connector, workspace-file, and knowledge-base targets canonically before current authorization; conceal trusted-workspace mismatches before mutation
  • route every knowledge_base Copilot branch through the shared Copilot application adapter
  • compose add_file, bulk knowledge-base delete, and bulk document delete as three fixed bounded application commands with no nested authorized use cases or generic multi-operation executor
  • migrate ordinary internal Knowledge JSON routes to defineInternalJsonRoute, including explicit session-only operations for legacy personal knowledge bases
  • keep multipart and large-body behavior declarative while preserving upload-session binding, byte-plane ordering, billing, analytics, and wire compatibility

Composition behavior

  • addWorkspaceFilesToKnowledgeBase owns canonical KB/file resolution, per-resource current authorization, provenance checks, presigned storage preparation, one bounded usage admission, sequential best-effort registration/processing, authoritative audit, and success effects
  • bulkDeleteKnowledgeBases and bulkDeleteKnowledgeDocuments each have a distinct semantic operation, a 100-item sequential best-effort cost policy, per-resource canonical authorization, explicit partial outcomes, audit, effects, and cancellation state
  • searchKnowledge remains one semantic query operation with an explicit post-parse cost policy and now returns canonical KB presentation metadata, removing the redundant pre-search read
  • cancellation stops between mutations; already-completed items are audited and effected before the Copilot adapter rethrows the user stop
  • if a later batch item hits an unknown infrastructure failure, completed items are audited and effected before the original failure propagates to the adapter and is rendered generically to the model

Trusted identity

  • Copilot identity comes only from the shared server-authored execution context; model workspace, user, operation, and resource fields are requested targets only
  • executor identity is signed from trusted workflow/user/execution fields; missing scope fails before transport and model _context cannot supply it
  • session, API-key, and delegated callers re-check current workspace authorization at the application boundary
  • billing ownership remains attribution only and never supplies authorization identity

Copilot tool inventory

  • migrate: create, get, and update now enter exclusively through executeCopilotKnowledgeUseCase
  • migrate: query uses one knowledge.search command and its canonical presentation metadata
  • migrate: add_file uses one bounded knowledge.documents.add_workspace_files command
  • migrate: delete uses one bounded knowledge.bulk_delete command
  • migrate: delete_document uses one bounded knowledge.documents.bulk_delete command
  • migrate: update_document
  • migrate: list_tags, create_tag, update_tag, delete_tag, get_tag_usage
  • migrate: add_connector, update_connector, delete_connector, sync_connector
  • defer: none

Internal Knowledge route inventory

  • migrate: /api/knowledge and /api/knowledge/[id]
  • migrate: /api/knowledge/[id]/restore
  • migrate: /api/knowledge/search
  • migrate: /api/knowledge/[id]/documents and /api/knowledge/[id]/documents/[documentId]
  • migrate: /api/knowledge/[id]/documents/[documentId]/chunks and /api/knowledge/[id]/documents/[documentId]/chunks/[chunkId]
  • migrate: /api/knowledge/[id]/documents/[documentId]/tag-definitions
  • migrate: /api/knowledge/[id]/documents/upsert
  • migrate: /api/knowledge/[id]/documents/uploads, /api/knowledge/[id]/documents/uploads/[uploadId], /parts, and /complete
  • migrate: /api/knowledge/[id]/tag-definitions, /api/knowledge/[id]/tag-definitions/[tagId], /api/knowledge/[id]/tag-usage, and /api/knowledge/[id]/next-available-slot
  • migrate: /api/knowledge/[id]/connectors, /api/knowledge/[id]/connectors/[connectorId], /documents, and /sync
  • non-goal: /api/knowledge/connectors/sync is a cron-authenticated capped scheduler/recovery job, not an ordinary caller resource operation
  • defer: none

v2 inventory

  • migrate / declarative: knowledge-base collection/detail, folders, document collection/detail, upload controls, and search
  • specialized declarative lifecycle: multipart document POST preserves IP abuse limit -> authentication -> rollout -> operation rate limit -> canonical admission/quota -> bounded multipart read -> storage transfer -> authorized registration -> presenter/effects
  • non-goal: v1 routes remain unchanged
  • defer: none

Compatibility and safety

  • GET /api/knowledge still accepts omitted workspaceId for personal knowledge bases and scope=active|archived|all
  • existing aliases, result shapes, partial-result envelopes, messages, upload size/type validation, billing attribution, analytics, PostHog behavior, and v2 503 mapping are preserved
  • PersonalApiKeysDisabledError remains 403; shared v2 concealment converts only resource authorization failures to 404
  • no DB migration, dependency change, fallback authorization, or bun.lock change

Type of Change

  • Bug fix

Testing

  • 23 focused Vitest files, 191 passing tests
  • Sim type-check
  • Biome on all touched files
  • bun run check:api-validation:strict
  • git diff --check

Checklist

  • Code follows project style guidelines
  • Self-reviewed changes and memory-loading boundaries
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 9, 2026 7:35am

Request Review

@cursor

cursor Bot commented Aug 8, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Large refactor of auth, billing, provenance, and connector validation at the HTTP boundary; behavior is intended to be preserved but regression risk is meaningful across many knowledge endpoints.

Overview
This PR replaces hand-written withRouteHandler knowledge API routes with defineInternalJsonRoute, wiring shared auth (internalKnowledgeSessionOrExecutorAuth), rate limits, and domain error policies while delegating work to lib/knowledge/application use cases (connectors, documents, chunks, tags, uploads).

Connectors, documents, chunks, and tag-definitions routes no longer inline Drizzle queries, checkKnowledgeBaseAccess, or orchestration calls in the route file; they map contract params/body to use-case input and present responses via helpers like toInternalKnowledgeConnector / toInternalKnowledgeDocument. Connector update still injects makeSourceConfigValidator at the route boundary for live sourceConfig validation with OAuth/API-key resolution.

Document and chunk handlers keep secret provenance behavior through renderResponse / createKnowledgePersistedResponse and billing attribution via resolveInternalKnowledgeBillingAttribution. Upload session routes switch to internalSessionAuth and drop requireKnowledgeDocumentUploadActor and knowledgeDocumentUploadErrorResponse from route-local utils; completion still runs PostHog / platform analytics only when a document is newly created.

Route-level Vitest suites for these handlers are removed from app/api/knowledge/** (coverage expected elsewhere per PR description).

Reviewed by Cursor Bugbot for commit c0d251f. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR centralizes Knowledge authorization around canonical resource resolution and trusted delegated execution scope.

  • Migrates internal and v2 Knowledge routes to shared declarative route adapters and application use cases.
  • Propagates server-authored workflow identity through internal Knowledge tool requests.
  • Adds bounded application commands for multi-resource document and knowledge-base operations.
  • Preserves upload, billing, audit, analytics, and response compatibility across migrated routes.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/lib/knowledge/application/authorization.ts Defines shared Knowledge delegation policy and canonical workspace authorization behavior.
apps/sim/lib/knowledge/application/contexts.ts Resolves knowledge bases and child resources to canonical workspace contexts before authorization.
apps/sim/lib/knowledge/application/chunks.ts Moves chunk operations behind application authorization while retaining rejection of unavailable secret provenance.
apps/sim/lib/api/server/routes/internal-json-route.ts Introduces the declarative internal JSON route lifecycle used by migrated Knowledge endpoints.
apps/sim/lib/api/server/routes/v2-body-lifecycle-route.ts Encapsulates v2 authentication, admission, body parsing, and application-use-case execution.
apps/sim/lib/copilot/tool-executor/executor.ts Supplies trusted user, workflow, workspace, and execution context when Copilot dispatches application tools.
apps/sim/tools/index.ts Generates delegated executor authentication for Knowledge ToolConfig HTTP requests.
apps/sim/lib/copilot/application/execute-knowledge-use-case.ts Adapts Copilot Knowledge operations to shared application use cases and trusted execution scope.
apps/sim/lib/knowledge/application/add-workspace-files.ts Composes canonical authorization, usage admission, file registration, processing, audit, and effects into one bounded command.
apps/sim/lib/knowledge/application/documents.ts Centralizes authorized document operations, including bounded bulk behavior and partial outcomes.
apps/sim/lib/knowledge/application/connectors.ts Moves connector reads, updates, deletion, document exclusion, and synchronization behind canonical authorization.
apps/sim/lib/knowledge/application/search.ts Performs Knowledge search as one authorized operation and returns canonical presentation metadata.

Sequence Diagram

sequenceDiagram
  participant Caller as Session/API key/Copilot
  participant Route as Knowledge route adapter
  participant Context as Canonical context resolver
  participant Authz as Workspace authorization
  participant UseCase as Knowledge application use case
  participant Store as DB/Object storage/Vector store

  Caller->>Route: Knowledge request
  Route->>Route: Authenticate trusted principal
  Route->>UseCase: Validated input and principal
  UseCase->>Context: Resolve requested resource
  Context-->>UseCase: Canonical workspace and resource
  UseCase->>Authz: Authorize principal against canonical scope
  Authz-->>UseCase: Current permission decision
  UseCase->>Store: Read or mutate authorized resource
  Store-->>UseCase: Result
  UseCase-->>Route: Application outcome
  Route-->>Caller: Compatible response envelope
Loading

Reviews (6): Last reviewed commit: "fix(knowledge): finalize partial batch e..." | Re-trigger Greptile

Comment thread apps/sim/lib/copilot/tools/server/knowledge/knowledge-base.ts
@TheodoreSpeaks
TheodoreSpeaks force-pushed the fix/v2-copilot-knowledge-scope branch from 5c45991 to 6c8a34e Compare August 8, 2026 23:41
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 6c8a34e. Configure here.

@TheodoreSpeaks
TheodoreSpeaks force-pushed the fix/v2-copilot-knowledge-scope branch from 6c8a34e to 79c5879 Compare August 9, 2026 01:32
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/api/contracts/knowledge/connectors.ts
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/knowledge/application/add-workspace-files.ts
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

1 issue from previous review remains unresolved.

Fix All in Cursor

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 0af5acf. Configure here.

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant