Feature/enhance authorization

[pikann]

Summary

Authorization remediation across the API and web app: finer-grained project permissions, per-instance access control for agents/environments, a fixed wildcard/nesting permission check, plugin nav/settings-tab permission gating, and a plugin-outbound SSRF fix.

Project settings permissions

• Split project.settings.task_types, project.settings.task_statuses, and project.settings.custom_fields out of tasks.write — redefining the task schema (types, statuses, custom fields) is now a separate capability from editing task content, so a role can be granted one without the others.
• Split views.read/views.write out of sprints.write (which it previously borrowed) — a saved view's own definition (create/update/delete/reorder) is now distinct from moving a task within a view, which stays on tasks.write.
• Added dedicated plugins.read/plugins.write — global plugin install/marketplace management previously reused users.write as a rough proxy.
• Updated built-in roles (Project Owner/Manager/Member/Viewer, and the global Admin/Editor/Viewer roles) to grant the new permissions.

Per-instance access control for agents and environments

• New access_mode (open/restricted) column on agents and environments, plus agent_access_grants / environment_access_grants tables, let a project admin restrict a specific agent or environment to a chosen subset of project members.
• This is a second, independent layer on top of the existing agents.*/environments.* permissions: those remain the ceiling ("can this member use agents/environments at all"), the new grants add the allow-list ("which specific ones"). Defaults to open, so this is a no-op for every existing agent/environment until an admin opts one into restricted.
• No bypass for agents.write/environments.write holders — configuring a restricted resource is kept separate from being allowed to use it, mirroring the existing environments.connect vs. environments.write split.

Wildcard/nesting permission check fix

• Reworked permission matching so a broader wildcard (e.g. project.settings.*) correctly covers its nested permissions, fixing cases where granting a wildcard didn't actually grant everything underneath it.

Plugin nav & settings-tab permission gating

• Sidebar nav items and project-settings plugin tabs are no longer hidden based on permissions; they're always shown and now render an explicit no-permission state, driven by a new requiredPermission field on plugin nav/extension-point registrations enforced on both project and admin plugin pages.
• Plugin routes with no explicitly-declared middleware now default to authn (require login) instead of optionalAuthn, so a route that forgets to declare its own middleware fails closed rather than being reachable anonymously.

Plugin outbound SSRF fix

• The paca.fetch HTTP client used for every installed plugin's outbound calls now goes through netguard.NewSafeHTTPClient, which pins the dial to the already-validated IP. Previously it re-resolved DNS independently at dial time, leaving a DNS-rebinding gap — the same vulnerability class as GHSA-cj3q-c44j-q8p9, just a different call site (the marketplace/installer clients already used netguard; this one hadn't been wired up).

Test plan

• go test ./... in services/api
• New/updated integration tests for task types/statuses/custom fields, views, agent/environment access grants, and permission response shape pass
• Manual: restrict an agent/environment to specific members and verify non-granted members are blocked while granted members and *.write holders (without a grant) are still blocked from use but can still configure it
• Manual: as a member without a plugin's requiredPermission, confirm the nav item/settings tab is visible but shows a no-permission state instead of the plugin UI
• Manual: verify a project-scoped *.* or project.settings.* grant now correctly covers its nested permissions

🤖 Generated with Claude Code

[pullfrog]

Important

This is a genuinely polished, well-tested authorization rework — but two findings read as must-address before merge (a restricted mode that global agents can chat around, and a migration that silently promotes every per-project role named "Admin"), plus a couple of scope questions worth confirming.

Reviewed changes

• Nested-wildcard permission matching — hasPermission in apps/web/src/lib/permissions.ts and apps/mcp/src/permissions.ts now tries every granted .* key as a segment-aligned prefix (matching the Go authorizer), so project.settings.* actually covers its leaves; dedicated regression tests on all three matchers, and dedupeGrantedPermissions/expandWildcardPermissions follow the same predicate.
• New permission namespaces & route re-keying — project.settings.{task_types,task_statuses,custom_fields}.* split out of tasks.*, views.* split out of sprints.*, global plugins.* split out of users.write, annotations.* surfaced in the project role editor; router, MCP tool map, and role editor all re-keyed consistently.
• Agent & environment access grants — access_mode (open/restricted) plus per-member grant tables, new RequireAgentAccess/RequireEnvironmentAccess middleware and service-layer HasAgentUsageAccess/HasEnvironmentUsageAccess gates on chat / browse / SSH keys / port forwards / terminal; per-caller access_granted response decoration; "visible but locked" UI (restricted badges, grant management tabs, pickers that filter ungranted resources).
• Membership implies projects.read — ListProjectPermissions/ListAgentProjectPermissions auto-grant projects.read to active members, and gain the previously-missing deleted_at IS NULL filter (a real stale-permission bugfix).
• Admin-role wildcard backfills — migration 000054 backfills the settings/views grants onto existing role rows; 000056 collapses per-project Admin rows to the bare *; CreateProject now seeds Admin with *.
• Permission-denied UX hardening — 4xx responses no longer retried by the query client, a shared NoPermissionState component, RouteErrorComponent distinguishes 403/404, and CatchBoundary wraps only the routed content; route loaders no longer prefetch permission-gated queries so pages degrade to a no-permission state instead of crashing.
• Auth & plugin hardening — refresh/annotation-refresh re-issue tokens with the freshly reloaded role (demotions now take effect without a logout), plugin-route default middleware closes optionalAuthn → authn, the plugin paca.fetch client now pins DNS through netguard.

⚠️ Restricting an environment doesn't gate every surface that exposes its contents

envAccess was chained onto folders / browse / SSH keys / port forwards / terminal-ticket, but three surfaces escape it on a restricted environment: POST /{environmentId}/stats-ticket still mints a live-usage telemetry WebSocket into the container (an "alternate access path" by the same rubric used to justify gating SSH keys), the un-gated GET /environments/{environmentId} response still carries the connection metadata (ssh_port + bastion context), and annotations nested under a restricted environment's port forwards are gated only on annotations.*/tasks.* (acknowledged in a comment at the port-forward detail route as deliberately out of scope). If stats-ticket staying ungated is intentional it's worth saying so explicitly — as written, a member the admin meant to lock out of a restricted environment still receives live telemetry from it.

Technical details

# Restricted environments: ungated surfaces leak contents

## Affected sites
- services/api/internal/transport/http/router/router.go — `envAccess` applied to folders/browse/ssh-keys/port-forwards/terminal-ticket, but NOT to `/stats-ticket` or the detail GET
- services/api/internal/transport/http/handler/environment_handler.go — GetEnvironment returns `ssh_port`/bastion host to any `environments.read` holder
- router.go annotations block — nested env annotations require only annotations.*/tasks.*

## Required outcome
- Decide and document the intended perimeter: either extend `RequireEnvironmentAccess` to stats-ticket (live telemetry from inside the container) and accept the SSH metadata exposure, or a code comment explaining why those two are deliberately outside the grant model (like the annotations comment already does)

## Open questions for the human
- Is the stats-ticket WebSocket considered "usage" of the environment, or an ops surface that lifecycle actions also represent?

ℹ️ Trigger-driven dispatch runs restricted agents with no grant check

TriggerTaskAssigned/TriggerDirectMessage/TriggerCommentMention/TriggerDescriptionWrite create conversations and publish triggers without calling hasAgentUsageAccess (the "no human actor" deferral documented at agent_service.go:2223-2236), and can attach a restricted default environment. So a member holding only tasks.write/automation.write can cause a restricted agent — and its restricted environment — to execute even without a grant. Output is protected (a non-granted member gets ErrConversationNotFound), but the run itself, and whatever the trigger's own inputs carry into it, is not. The deferral is documented and there's a test asserting it (TestGetConversationForAgent_RestrictedAgent_SystemTriggeredCurrent_SharedAudience_Allowed), so this reads as a deliberate trade-off — but it materially limits what restricted guarantees, and the PR description doesn't mention it.

ℹ️ The web's project permission map silently drops the membership-implied projects.read

GetMyProjectPermissions returns the raw role permissions map and does not apply the new membership-implies-projects.read rule, so now that the seeds/templates no longer list projects.read, the web's own project map (which feeds useProjectPermissions) reports it as denied for Editor/Member/Viewer while the backend grants it. Nothing in the UI currently gates on project-scoped projects.read, so this is latent today — but the two sides of the same role now disagree in the deny direction. Cheap to align by appending the implied key in GetMyProjectPermissions alongside ListProjectPermissions.

ℹ️ Nitpicks

• authorizeConversationAccess's new doc comment says a memberID == uuid.Nil sentinel "correctly fails" the restricted-agent gate "— there is no project_members row for the zero UUID to ever match", but hasAgentUsageAccess (agent_service.go:1198) returns true for nil memberID by design (the intended machine-path sentinel, covered by a dedicated test). The comment contradicts the behavior and could invite a future "fix" that breaks the MCP read_conversation path.
• The * Admin seed doesn't round-trip through the role editor: opening the Admin role (now {"*": true}) and saving re-emits enumerated wildcards via normalizePermissionsToWildcards, silently undoing the future-proofing 000056 was written for. Supporting * in the editor (or a warning when a role carries it) would keep the two consistent.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

ℹ️ The follow-up commit cc7d543 is a clean, self-contained UX fix — no new issues found in this delta. The findings from the previous review (head 0122497) are unaffected and still open.

Reviewed changes (delta over 0122497 only)

• Loading-aware permission hooks — usePermissions/useProjectPermissions now expose isLoading, and the project/admin list pages (conversations, settings custom-fields/roles/task-statuses/task-types, global agents/roles/users, project agents/automation/environments, team) gate noPermission on !isPermissionsLoading and fold the permission fetch into their skeleton state. This fixes the NoPermissionState flash that occurred while the permission map was still in flight (an unloaded map reads identically to a confirmed denial).
• Hook regression tests — use-project-permissions.test.tsx covers the loading-state contract (isLoading passthrough) and the granted-only-keys filter passed to hasPermission, with a real assertion on the filtered argument.

The isLoading threading is correct across call sites: the conversations layout explicitly guards global chat (!!projectId && ...), and the team page correctly waits on both global and project permission sources. One noted range-diff artifact — a duplicate isLoading binding in the global-roles/users pages — turned out to be a diff-rendering artifact; the actual files rename it to isDataLoading and compile cleanly.

None of the previous review's findings (restricted-global-agent gap via global chat, migration 000056 Admin escalation, net-new views.write for members, restricted-env stats-ticket/SSH-metadata surface, trigger-dispatch deferral) are touched by this commit — expected, since it is limited to web loading UX.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

ℹ️ No new issues in this delta — commit e9fa8d9 is a clean, well-tested fix, and the prior review findings are unaffected and remain open.

Reviewed changes (delta over cc7d543, commit e9fa8d9 only)

• Plugin-route middleware round-trip fix — dropped omitempty from PluginRoute.Middlewares' json tag so the load-bearing nil-vs-explicit-empty distinction survives the JSONB manifest marshal/unmarshal cycle. Previously a route declared with middlewares: [] had its key dropped on marshal, loaded back as nil, and silently fell into the (now auth-required) default policy on every request after its first store/load — flipping an explicitly-public route to gated.
• Regression test — TestPluginRouteMiddlewares_SurvivesManifestRoundTrip runs the full marshal→unmarshal round trip for nil / explicit-empty / populated middleware lists, with the explicit-empty case failing against the old omitempty code.
• Cosmetic realignment — whitespace-only field alignment in bootstrap/app.go, agent/entity.go, agent_dto.go, and environment_dto.go.

Verified locally: go test ./internal/domain/plugin/*.go and the plugin handler route/middleware tests pass. One deploy-time observation worth carrying: the fix only takes effect for manifests marshaled after upgrade — already-installed plugins whose manifest was stored under omitempty keep the auth-gated default until the plugin is re-saved. That direction is fail-closed and consistent with the PR's hardening, so no code change is needed, just awareness.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

ℹ️ No new issues in this delta — commit ce5fa9b is a small, well-reasoned, well-tested follow-up. The findings from the previous reviews are unaffected and remain open.

Reviewed changes (delta over e9fa8d9, commit ce5fa9b only)

• Restored 404 semantics for unmatched plugin paths — routeMiddlewares now returns no middleware when no manifest route matched the request's method+path, instead of letting those paths fall into the new authn fail-closed default (which turned them into 401s). The request reaches the plugin's own WASM router unauthenticated, which 404s it — the pre-PR behavior, pinned by the existing TestE2EPluginRuntime_APICall_UnmatchedPath_PluginReturns404. Declared-but-unprotected routes keep the authn + requireFreshPassword default, so the fail-closed hardening for the manifest's own routes is unchanged.
• Regression test for the nil-route case — TestRouteMiddlewares_NilVsEmpty gains a case asserting routeMiddlewares(nil) applies no middleware, documenting the unmatched-path contract at the unit layer alongside the E2E test.

I traced the full path (ProxyRequest → matchPluginRoute → applyPluginRouteMiddlewares → WASM HandleRequest): projectMemberParam(nil) is nil-safe, and the anonymous-dispatch behavior this restores is exactly what master's optionalAuthn default already did for unmatched paths, so there's no net-new exposure versus the pre-PR state. Verified the handler and route-middleware tests pass locally.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

ℹ️ No critical issues — this delta is a clean, well-tested read-permission cleanup, with one silent behavior change worth confirming inline.

Reviewed changes (delta over ce5fa9b, commit 4fe14367 only)

• Dropped the three project.settings.*.read permissions entirely — viewing task types/statuses/custom fields is now implied by tasks.read (there's no boundary in seeing the schema that isn't already crossed by seeing the tasks that use it). Removed consistently across the Go constants, web role editor, MCP tool map, i18n locales, and integration-test fixtures; grep confirms no dangling references to the deleted keys.
• Narrowed default Editor/Viewer grants — PROJECT_MEMBER loses the PermissionProjectSettingsAll wildcard, the per-project Editor/Viewer seeds drop the settings grants, migration 000054 is rewritten in place to grant only views.*/views.read, and PROJECT_VIEWER gains views.read in defaults.
• Regression tests — TestDefaultProjectRoles_ProjectMemberHasNoSettingsWritePermissions and TestCreate_EditorRoleHasNoSettingsWritePermissions pin the resulting grant sets (both fail against the pre-delta defaults); router GET gates for the three schema routes now require tasks.read only, while the write routes stay single-gated on project.settings.*.write.
• Verified locally: go build ./..., authz/project/plugin unit suites, and go test ./test/integration all pass.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

Important

The SSH-key gate and the trigger-path access checks are correct and close real holes — but the new deny paths ship with zero regression coverage, and every prior fix in this PR got one.

Reviewed changes (delta over the prior pullfrog review — commit e936ce53 only)

• SSH key registration/removal re-gated onto environments.connect — POST/DELETE /{environmentId}/ssh-keys in router.go move off environments.write, and the web connect page swaps its canWrite → canConnect in SSHKeysManager/SSHConnectTab, because a registered key is just another root-shell path (same class as the terminal ticket). Backend and UI stay aligned; no default role regresses (Member/Editor hold write+connect, Viewer has neither).
• Restricted grants now enforced on trigger-driven dispatch — new authorizeConversationTrigger helper runs both hasAgentUsageAccess and HasEnvironmentUsageAccess on TriggerTaskAssigned/TriggerDirectMessage/TriggerCommentMention/TriggerDescriptionWrite for human actors, while the unattended automation branch (nil triggeredByMemberID from automation_consumer) keeps the documented "no human actor" deferral. This closes the human-actor half of the earlier trigger-dispatch finding.
• Consistency pass — TriggerDescriptionWrite now threads the fetched agent through instead of discarding it, the three other triggers resolve the agent via deleted-row-filtered FindAgentByID, and the PermissionEnvironmentsConnect/hasAgentUsageAccess doc comments were rewritten to name both kinds of nil-membership caller.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

ℹ️ No critical issues — the denial-error surfacing is wired correctly end to end; two minor nits are up for a follow-up.

Reviewed changes (delta over e936ce53 — commit bc47a65 only)

• Surfaced denied-chat-session errors on all three project chat surfaces — new chatSessionAccessDeniedKey classifies dispatch failures as AGENT_ACCESS_RESTRICTED, ENVIRONMENT_ACCESS_RESTRICTED, or a generic 403, and conversation-view.tsx / new-conversation-thread.tsx / ai-chat-float.tsx now render the translated message via a local sendError state + ConversationErrorBox instead of relying on assistant-ui's onNew, which drops thrown errors as unhandled promise rejections.
• Added the two restrict codes to the web ApiErrorCode map — the exact codes the RequireAgentAccess/RequireEnvironmentAccess middleware and the chat-session service gates emit (router.go:821-829, agent_service.go:2216/2244), so the specific "ask an admin" messages fire on the paths that actually gate.
• Localized the new messages across all nine locales — agents.conversationView.{agentAccessRestricted, environmentAccessRestricted, chatNoPermission, conversationEnded, textOnlyMessage} verified in key parity on en/es/fr/ja/ko/pt-BR/ru/vi/zh-CN.

The resume path (sendConversationMessage → authorizeConversationAccess) returns 404 ErrConversationNotFound rather than a 403, which the classifier correctly leaves alone — and it's unreachable anyway since the conversation itself won't load for a non-granted member. Verified the affected unit suites pass locally and the new codes round-trip through getApiErrorCode's known-codes filter.

ℹ️ Nitpicks

• No unit coverage for chatSessionAccessDeniedKey or the two new ApiErrorCode values — a pure classifier whose ordering contract (specific codes before the generic 403, null for non-403) is exactly what drifts, and the rest of this PR's fixes each shipped with regression tests.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

Important

The 6f28b6e7 delta closes most of the prior findings cleanly and with good tests — but the new global-chat gate is bypassable through an existing conversation's resume path, so a restricted global agent remains executable by anyone who chatted with it before the restriction. That contradicts the gate's own "fails every global-chat caller closed" contract and should be closed (or explicitly grandfathered) before merge.

Reviewed changes — delta over bc47a65 (commit 6f28b6e7 only, 21 files):

• *-wildcard role editor round-trip — ProjectRoleFormDialog gained isFullAccess tracking so a role stored as the bare {"*": true} (the migration-000056 "Admin") saves back as the wildcard when untouched, converting to today's enumerated set only once a toggle actually changes; badge + explanation UI with copy across all nine locales, plus three new vitest cases.
• Global-chat restriction enforced fail-closed — new requireGlobalAgentOpen gate on ListGlobalChatSessions/StartGlobalChatSession/SendGlobalChatMessage (preserving the validateOnBusy-before-lookup ordering), each pinned by its own _RestrictedAgent_Rejected test asserting no DB writes occur.
• Trigger deny-path coverage — TestTrigger{TaskAssigned,DirectMessage,CommentMention}_RestrictedAgent_NonGrantedMember_Rejected pin authorizeConversationTrigger's deny side, and activity_service.go now logs a comment-mention denial instead of silently dropping it while comment posting still succeeds.
• GetMyProjectPermissions parity — the web-facing project permission map now copies the role map and appends the membership-implied projects.read, matching backend ListProjectPermissions (no aliasing of the shared map); nil-permission and agent callers covered by updated tests.
• Restricted-environment surface closure — stats-ticket moved under RequireEnvironmentAccess, and toEnvironmentResponseForCaller strips ssh_port from restricted non-granted responses on list and detail, with three handler tests including an open-mode no-op guard.
• Doc-comment corrections — authorizeConversationAccess's nil-sentinel contract, chatSessionAccessDeniedKey's setSendError/ConversationErrorBox mechanism, and the net-new member views.write capability are now all described accurately.

ℹ️ Floating global chat widget has no way to render the new denial

The home/admin-page float (ai-chat-float-global.tsx) still wraps startGlobalChatSession/sendGlobalChatMessage in a bare try/finally — no catch, no chatSessionAccessDeniedKey. With this commit's gate, picking a restricted global agent there and sending produces the 403 as an unhandled onNew rejection — the exact failure the setSendError/ConversationErrorBox work in bc47a65 was built to fix on the sibling surfaces — while useGlobalAgentPicker lists restricted agents without filtering (unlike the project-scoped useAgentPicker, which drops ungranted restricted agents). The same endpoints run through the classifier and render the denial correctly on conversation-view.tsx and new-conversation-thread.tsx, so this is float-only roughness: routing its dispatch errors through the classifier, or pre-filtering the global picker, would keep the "visible but locked" contract consistent.

ℹ️ Nitpicks

• TriggerDescriptionWrite is the fourth authorizeConversationTrigger path and its deny side (agent_service.go:3153) still has no _RestrictedAgent_NonGrantedMember_Rejected mirror — the three sibling triggers each got one in this commit.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

The e4066e57 delta directly closes the last open finding from the prior review — the existing-conversation resume path that could keep a restricted global agent executing — with a clean, well-testified fix. No new issues discovered.

Reviewed changes — delta over 6f28b6e7 (commit e4066e57 only, 2 files):

• Closed the global-conversation resume gap — SendGlobalConversationMessage now gates through a new agent-in-hand requireAgentOpen(agent) helper (extracted from requireGlobalAgentOpen, which now delegates to it) immediately after the already-fetched FindAgentByID and before either dispatch branch, so a conversation created before an agent was restricted can no longer keep it executing. Stop/Pause/Heartbeat deliberately remain ungated for in-flight-run cleanup, matching the thread's suggested outcome.
• Committed the fourth trigger deny-path test — TestTriggerDescriptionWrite_RestrictedAgent_NonGrantedMember_Rejected mirrors the three sibling authorizeConversationTrigger deny tests (fails via the mocked createConversation fatal if the gate is ever removed), and TestSendGlobalConversationMessage_RestrictedAgent_Rejected asserts no claim/dispatch happens for a restricted agent.

Verified the agent service suite passes with the new cases: go test ./internal/service/agent -test.run=RestrictedAgent — all 11 restricted-agent tests pass, including both new ones.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

The c4cf3cec delta is CI-only: the race-detector test run in api-pr-ci.yml gets a per-test -timeout bump from 60s to 120s with a clear rationale comment. Directly closes the flaky-timeout watch item from the prior review (the wazero WASM plugin concurrency test timing out at 60s on a contended runner, green locally every time). The comment's claims check out — the 120s budget matches acp-bridge-pr-ci.yml:107, and the 15-minute job cap leaves ample headroom. No new issues found.

Reviewed changes — delta over e4066e57 (commit c4cf3cec only, 1 file):

• Bumped the race-detector test timeout — api-pr-ci.yml raises the go test -race per-test budget from 60s to 120s, with a comment explaining the WASM JIT-compiling plugin fixtures edge the old budget on slower/contended CI runners (internal/platform/plugin's suite runs ~45s with near-zero headroom), matching the existing acp-bridge budget; integrity-checked coverage upload left untouched.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}