Skip to content

fix(knowledge): apply knowledge-base access checks consistently across auth types - #6426

Merged
waleedlatif1 merged 2 commits into
stagingfrom
worktree-kb-tag-definitions-authz
Aug 8, 2026
Merged

fix(knowledge): apply knowledge-base access checks consistently across auth types#6426
waleedlatif1 merged 2 commits into
stagingfrom
worktree-kb-tag-definitions-authz

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Run the knowledge-base access check on the acting user for every auth type on /api/knowledge/[id]/tag-definitions, instead of only for browser sessions
  • Use read access for GET and write access for POST, matching the sibling knowledge routes
  • Require an acting user to be present before authorizing
  • Thread the acting user through the KB tag schema enrichers so their request carries the identity the route authorizes

Type of Change

  • Bug fix

Testing

Added route tests covering both auth types and the granted/denied/not-found/no-acting-user paths, plus enricher tests. Verified each new test fails against the previous behavior. Full app/api/knowledge + tools suites pass (550 tests), type-check, lint:check, and check:api-validation:strict clean.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • 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 8, 2026 7:11pm

Request Review

@cursor

cursor Bot commented Aug 8, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Changes authorization on a knowledge API route and removes trust of internal JWT without per-user KB checks—a security fix that could break workflow tool schema enrichment if acting user is not threaded everywhere enrichSchema is invoked.

Overview
Fixes an authorization gap on /api/knowledge/[id]/tag-definitions: internal JWT callers were previously trusted without a knowledge-base check, and GET incorrectly required write access.

GET now requires an acting userId, runs checkKnowledgeBaseAccess for every auth type (read, not write), and returns 401/403/404 when appropriate. POST applies the same userId requirement and always runs checkKnowledgeBaseWriteAccess, aligning with sibling knowledge routes.

KB tag schema enrichers now receive WorkflowToolExecutionContext and call buildAuthHeaders(context.userId) when fetching tag definitions; enrichment is skipped when no acting user is present. SchemaEnrichmentConfig.enrichSchema is extended to pass that context from createLLMToolSchema.

Route and enricher tests cover session vs internal JWT, granted/denied/not-found, and missing acting user.

Reviewed by Cursor Bugbot for commit 3419e4f. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR consistently authorizes knowledge-base tag-definition requests using the acting user and propagates that identity through tool-schema enrichment.

  • Requires an acting user for session and internal-JWT requests.
  • Applies read authorization to GET and write authorization to POST.
  • Passes workflow execution context into parameter schema enrichers.
  • Adds route and enricher coverage for authorized, denied, missing, and unauthenticated paths.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/app/api/knowledge/[id]/tag-definitions/route.ts Applies acting-user authorization consistently, using read access for GET and write access for POST.
apps/sim/app/api/knowledge/[id]/tag-definitions/route.test.ts Covers session and internal-JWT authorization outcomes, with the previously requested readonly fixture assertions present.
apps/sim/tools/params.ts Passes workflow execution context to parameter-level schema enrichers.
apps/sim/tools/schema-enrichers.ts Uses the acting user when requesting KB tag definitions and safely skips enrichment when that identity is absent.
apps/sim/tools/schema-enrichers.test.ts Verifies acting-user header propagation and identity-less enrichment behavior.
apps/sim/tools/types.ts Extends the parameter schema-enrichment contract with workflow execution context.

Sequence Diagram

sequenceDiagram
  participant Tool as Workflow Tool
  participant Enricher as KB Schema Enricher
  participant Auth as Internal Auth Builder
  participant Route as Tag Definitions Route
  participant Access as KB Access Check
  participant Tags as Tag Service
  Tool->>Enricher: Enrich schema (KB ID, execution context)
  alt Acting user is present
    Enricher->>Auth: Build headers for user ID
    Auth-->>Enricher: Internal JWT headers
    Enricher->>Route: GET tag definitions
    Route->>Access: Check read access for acting user
    alt Access granted
      Access-->>Route: Allowed
      Route->>Tags: Get tag definitions
      Tags-->>Route: Definitions
      Route-->>Enricher: 200 with definitions
      Enricher-->>Tool: Enriched schema
    else Access denied
      Access-->>Route: Forbidden or not found
      Route-->>Enricher: 403 or 404
      Enricher-->>Tool: No enrichment
    end
  else Acting user is absent
    Enricher-->>Tool: Skip enrichment
  end
Loading

Reviews (2): Last reviewed commit: "chore(tests): mark tag-definition test f..." | Re-trigger Greptile

Comment thread apps/sim/app/api/knowledge/[id]/tag-definitions/route.test.ts
…s auth types

The tag-definitions route only ran its knowledge-base access check for browser
sessions, skipping it for internal JWT callers. Authorize on the acting user
for every auth type instead — read access for GET, write access for POST — and
require an acting user to be present, matching the sibling knowledge routes.

Thread the acting user through the KB tag schema enrichers so their request
carries the identity the route now authorizes.
@waleedlatif1
waleedlatif1 force-pushed the worktree-kb-tag-definitions-authz branch from 7c47366 to 3419e4f Compare August 8, 2026 19:11
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

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 3419e4f. Configure here.

@waleedlatif1
waleedlatif1 merged commit 3726bd2 into staging Aug 8, 2026
24 checks passed
@waleedlatif1
waleedlatif1 deleted the worktree-kb-tag-definitions-authz branch August 8, 2026 19:14
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