fix(security): scope activity feed and log-stream WS to the caller workspace - #576
Merged
Merged
Conversation
…rkspace Two linked cross-tenant leaks found via internal security review (SECURITY). A) GET /api/activity returned the whole deployment's activity feed with no workspace filter and emitted the logged action params jsonb verbatim. Since logAction spread request bodies into params, plaintext secrets and other tenants' resource ids leaked to any authenticated user (also an id oracle for cross-tenant IDORs). The feed's CTEs also interpolated since/userId/resourceType into sql.raw strings. B) The workflow-run and persistent-agent log-stream WS handlers streamed any run/agent by id with no workspace check, exposing other tenants' agent output live. Fixes: - Add optio_actions.workspace_id (+ index) via migration 1785714869_optio_actions_workspace_id; logAction now stamps the caller's workspace and reduces params to an explicit non-secret allowlist (ids/labels) instead of storing request bodies. - /api/activity: requireRole(member) (viewers blocked); action + task_event CTEs scoped to the caller's workspace (task_event via join to tasks); global auth/infra CTEs and legacy null-workspace rows restricted to admins; since/userId/resourceType parameterized; response params reduced to the same allowlist so legacy rows can't leak. - New ws/ws-authz.assertWorkspace helper (closes 4403 on null-normalized mismatch, preserving auth-disabled null==null); applied to the workflow-run, persistent-agent, and pr-review log-stream handlers. - Tests: activity route (workspace scoping + secret-param stripping + viewer 403), optio-action-service (workspaceId persistence + allowlist), and ws-authz (helper + handler 4403). logAction sites in the connections/workflows/persistent-agents routes are owned by other work and left unstamped; those rows keep workspace_id NULL and are therefore admin-only in the feed (deny-by-default).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SECURITY — cross-tenant data leaks
Two linked HIGH-severity cross-tenant leaks, confirmed by an internal security review. No exploit payloads below.
Leak A — deployment-wide activity feed leaking plaintext-secret params
GET /api/activityreturned the whole deployment's activity feed with no workspace filter — its CTEs filtered only on time/user/resourceType. Worse, theactionbranch emittedoptio_actions.params(the logged action params jsonb) verbatim, andoptioActionService.logActionspread request bodies intoparams(...req.body,...input). So nested plaintext secrets (connection configs, webhook signing secrets, credentials) and other tenants' resource ids leaked to any authenticated user. This feed was also the "id oracle" that made cross-tenant IDORs exploitable. The CTEs additionally interpolatedsince/userId/resourceTypeintosql.rawstrings.Leak B — unscoped log-stream WebSockets
ws/workflow-run-log-stream.tsandws/persistent-agent-stream.tsstreamed any workflow-run / persistent-agent log by id with no workspace check, exposing other tenants' agent output live.Fixes
Data model
1785714869_optio_actions_workspace_id.sql: addsoptio_actions.workspace_id(+ index). Existing rows keepworkspace_id = NULL, treated as operator/legacy (admin-only in the feed).Write path (
optio-action-service.ts)logActionnow recordsworkspaceIdand reducesparamsto an explicit non-secret allowlist (resource ids, human-readable labels/types, a few scalar flags) — request bodies/secrets are dropped at write time, not merely redacted by key name.workspaceIdat ~39 tenant-resourcelogActioncall sites fromreq.user?.workspaceId ?? null.Read path (
routes/activity.ts)preHandler: [requireRole("member")]— viewers can no longer read the audit trail.oa.workspace_id;task_eventCTE scoped viaJOIN tasksontasks.workspace_id. Members see strictly their own workspace; admins additionally see legacy null-workspace rows.auth_event/pod_health_eventsCTEs have no tenant column (deployment-global) — restricted to admins, omitted for members.since/userId/resourceTypenow parameterized (no moresql.rawinterpolation).params— reduced to the same allowlist, so even legacy rows written before this change can't leak their full jsonb.WebSockets (
ws/ws-authz.ts— new)assertWorkspace(socket, userWorkspaceId, resourceWorkspaceId)closes 4403 unless the null-normalized workspaces match.null == nullpasses, preserving auth-disabled dev (synthetic null-workspace user) behavior.getWorkflow(run.workflowId)), before any streaming/catch-up.Tests
routes/activity.test.ts: a member is scoped to their own workspace (bound param is the caller's only; noIS NULLbranch), an admin additionally sees null-workspace rows, secret-bearing params are stripped from the response, viewers get 403.services/optio-action-service.test.ts:logActionpersistsworkspaceIdand stores only allowlisted keys (secrets/bodies dropped).ws/ws-authz.test.ts: helper (match / null==null / cross-workspace 4403 / scoped-vs-null) plus handler-level tests that a cross-workspace socket is closed 4403 and never subscribes.Verification
apps/apitypecheck + full workspace typecheck (12 packages) ✓bash scripts/check-migration-prefixes.sh✓apps/apiunit suite: 2180 tests ✓ws-log-streame2e still green (auth-disabled null==null streams) ✓pnpm format:check✓Scope note
logActionsites in theconnections/workflows/persistent-agentsroutes are owned by other in-flight work and were intentionally left unstamped. Those rows keepworkspace_id = NULLand are therefore admin-only in the feed (deny-by-default) until stamped — no cross-tenant exposure, and secrets are already stripped by the centralized write-time allowlist regardless of the call site.