Skip to content

fix(knowledge): validate tag slots and share the tag-name length limit - #6438

Closed
waleedlatif1 wants to merge 1 commit into
stagingfrom
kb-tag-authz-followup
Closed

fix(knowledge): validate tag slots and share the tag-name length limit#6438
waleedlatif1 wants to merge 1 commit into
stagingfrom
kb-tag-authz-followup

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Validate the tag slot on write: it reached the DB unchecked (the contract types it as a plain string, the column is text with a types-only Drizzle enum, and the service casts before inserting), so an unknown slot inserted a row nothing can read
  • Create route checks the slot against its declared field type using the existing isValidSlotForFieldType, which also closes a slot/field-type mismatch
  • Bulk document route checks slot validity only — that route also renames existing definitions, which resend whatever pair is already stored, so a legacy mismatched row stays fixable
  • Promote the tag display-name cap to a shared constant and apply it on every write path: both contracts, both modals, and the copilot create/update tools (model-generated names bypassed the contract entirely)
  • Drop two local copies of FIELD_TYPE_LABELS in favour of the shared one two other components already import

Notes

  • The display-name cap was already 100 on the bulk document route; this shares that number rather than inventing one. The internal create route previously had no cap, so a >100-char name posted there now 400s
  • No new helper: the slot checks reuse isValidSlotForFieldType / getFieldTypeForSlot from lib/knowledge/constants

Type of Change

  • Bug fix

Testing

Added route tests for unknown slot, slot/field-type mismatch, unsupported field type, and the display-name cap; verified each fails without its guard. app/api/knowledge + tools + copilot knowledge suites pass, 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)

The create-tag route validates fieldType at runtime because the contract keeps
it a plain string. Its sibling bulk route already explains why; without the same
note here the check reads as redundant with the contract and invites removal.
@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 8:32pm

Request Review

@cursor

cursor Bot commented Aug 8, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Comment-only change with no behavior, data, or auth impact.

Overview
Adds an inline comment on the knowledge-base create tag definition route (POST tag-definitions) explaining the defense-in-depth check that rejects fieldType values outside SUPPORTED_FIELD_TYPES after contract parsing.

The note clarifies why that guard stays at the route layer: tightening the API contract to the field-type enum would cascade into UI form state types, so validation uses a plain string in the contract and the runtime includes check enforces allowed types instead.

Reviewed by Cursor Bugbot for commit 3026e3a. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds explanatory documentation for the existing runtime field-type validation in the tag-definition creation route.

  • Clarifies why the contract retains a plain-string field type.
  • Documents why the supported-field-type check uses a widened readonly-string cast.

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 Adds a statement-level documentation comment explaining the existing validation and cast without changing runtime behavior.

Reviews (2): Last reviewed commit: "chore(knowledge): document the tag-defin..." | Re-trigger Greptile

@waleedlatif1 waleedlatif1 changed the title chore(knowledge): document the tag-definition field-type runtime check fix(knowledge): validate tag slots and share the tag-name length limit Aug 8, 2026
@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 3026e3a. Configure here.

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