feat(skills): permissions layer#5705
Conversation
…t staging Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # scripts/check-api-validation-contracts.ts
Same DDL as the dropped 0261 (skill_member table, enums, indexes, skill.workspace_shared) plus the hand-written write-user backfill, renumbered after staging's 0261. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
# Conflicts: # apps/sim/app/api/mothership/execute/route.ts # apps/sim/executor/handlers/agent/skills-resolver.test.ts # apps/sim/lib/copilot/chat/payload.ts # apps/sim/lib/copilot/chat/process-contents.test.ts # apps/sim/lib/copilot/chat/process-contents.ts # apps/sim/lib/copilot/vfs/workspace-vfs.ts # apps/sim/lib/mothership/inbox/executor.ts # apps/sim/lib/mothership/skills.test.ts # apps/sim/lib/mothership/skills.ts # packages/db/migrations/meta/0262_snapshot.json # packages/db/migrations/meta/_journal.json # scripts/check-api-validation-contracts.ts
Staging claimed 0262 (strong_storm); same DDL plus the hand-written write-user backfill, renumbered on the merged snapshot chain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t staging Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # scripts/check-api-validation-contracts.ts
Staging claimed 0263 (workflow_fork_sync_excluded); same DDL plus the hand-written write-user backfill, renumbered on the merged snapshot chain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR SummaryHigh Risk Overview API: Enforcement applies on interactive runs ( UI: Gallery opens Reviewed by Cursor Bugbot for commit 6bd6c8f. Configure here. |
Greptile SummaryThis PR adds per-skill permissions and a new skill management page. The main changes are:
Confidence Score: 4/5Agent execution can bypass skill permissions when its enforcement flag is false or absent.
apps/sim/executor/handlers/agent/skills-resolver.ts and its execution callers
|
| Filename | Overview |
|---|---|
| apps/sim/lib/skills/access.ts | Adds effective role resolution, explicit grants and denies, member listing, update checks, and workspace-removal cleanup. |
| apps/sim/app/api/skills/[id]/members/route.ts | Adds authenticated member listing, grants, role changes, removals, and transactional last-admin protection. |
| apps/sim/app/api/skills/route.ts | Adds skill visibility and administrator checks to list, update, and delete operations. |
| apps/sim/executor/handlers/agent/skills-resolver.ts | Makes skill permission filtering conditional, leaving executions without the enforcement flag unrestricted. |
| packages/db/migrations/0264_tough_steel_serpent.sql | Adds skill sharing and membership storage with an idempotent access backfill. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Agent execution] --> B{actor.enforce is true}
B -- Yes --> C[Load user skill access]
C --> D[Filter skills with canUseSkill]
B -- No or unset --> E[Return no access context]
E --> F[Skip skill permission checks]
F --> G[Restricted skill content can load]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Agent execution] --> B{actor.enforce is true}
B -- Yes --> C[Load user skill access]
C --> D[Filter skills with canUseSkill]
B -- No or unset --> E[Return no access context]
E --> F[Skip skill permission checks]
F --> G[Restricted skill content can load]
Reviews (1): Last reviewed commit: "fix lint" | Re-trigger Greptile
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6bd6c8f. Configure here.
| joinedAt: entry.joinedAt ? entry.joinedAt.toISOString() : null, | ||
| })) | ||
|
|
||
| return NextResponse.json({ members }) |
There was a problem hiding this comment.
Revoked members exposed to non-admins
Low Severity
The skill members GET handler returns the full roster from listSkillMembers, including revoked deny markers, to any caller with skill access. The detail UI only shows the Removed section to skill admins, so non-admin members can still learn who was explicitly denied via the API.
Reviewed by Cursor Bugbot for commit 6bd6c8f. Configure here.
| ): Promise<SkillAccessForUser | null> { | ||
| if (!actor?.enforce || !actor.userId) return null | ||
| return getSkillAccessForUser(workspaceId, actor.userId) | ||
| } |
There was a problem hiding this comment.
Disabled Flag Bypasses Skill Access
When an execution supplies a user ID but leaves actor.enforce false or unset, this branch skips the permission lookup and downstream resolvers treat every workspace skill as accessible. Non-interactive runs can therefore load metadata and content for restricted or revoked skills; skill authorization must not depend on the credential-enforcement flag.


Summary
Adds credential-style per-skill permissions — Admin/Member roles with derived workspace-admin access, a workspace-shared toggle with per-member restriction/deny, enforced across API, workflow runs, and the mothership (VFS/mentions/manage_skill), with an idempotent backfill preserving today’s access.
Replaces the edit modal with a full-page skill detail (/skills/[skillId]) matching the integrations layout: Share, Save with unsaved-changes guard, Access toggle, and member roster with restore.
Type of Change
Testing
Tested manually
Checklist