Harden workspace-scoped route access

[niko4244]

Hardens workspace-scoped API route access by removing silent workspace fallback, enforcing fail-closed workspace context, restricting agent-scoped API keys to their own heartbeat/memory and assigned task surfaces, and adding focused regression coverage for workspace-scope and same-workspace overreach cases.

Scope:

• Clean one-commit cherry-pick onto builderz-labs/main
• Commit: 1e09f7e Harden workspace-scoped route access
• Diff: 17 files changed
• Old oversized PR Harden workspace-scoped route access #667 should be closed and replaced by this PR

Security changes:

• Missing workspace context now fails closed on reviewed route paths
• Removed fallback to workspace 1 from reviewed routes/webhook bookkeeping
• Agent-scoped API keys are restricted to their own heartbeat and memory routes
• Agent-scoped API keys are restricted to their own assigned task/comment/queue surfaces
• Webhook CRUD uses workspace-scoped access

Validation note:
This branch is clean and isolated, but full repo validation currently fails on builderz-labs/main before this patch. The same failures are present on untouched upstream main and are unrelated to this PR.

Pre-existing upstream validation failures:

• pnpm typecheck: FAIL due to .next/dev/types/validator.ts missing-module errors and existing system-monitor-panel.tsx type issues
• pnpm test: FAIL in existing OpenCode/config/path/reconciliation test suites
• pnpm lint: FAIL in existing unrelated panel files
• pnpm build: FAIL during type checking on the same pre-existing validator/type issues

Patch-specific finding:

• No validation failure was introduced by this workspace-scope hardening patch.

[niko4244]

Reviewer note:

This replaces oversized PR #667.

This PR is intentionally scoped to one clean cherry-picked commit:

• 1e09f7e Harden workspace-scoped route access

Security focus:

• Fail closed when workspace context is missing
• Remove workspace 1 fallback from reviewed route paths / webhook bookkeeping
• Restrict agent-scoped API keys to their own heartbeat and memory routes
• Restrict agent-scoped API keys to their own assigned task, comment, and queue surfaces
• Add focused route-security regression tests

Validation caveat:
Full repo validation is currently red on builderz-labs/main before this patch. I verified the same typecheck, test, lint, and build failures exist on untouched upstream main, so they are not introduced by this PR.

[liberathio]

Review (community / read-only) — Verdict: NEEDS-CHANGES ⚠️

The security intent is sound, the design (fail-closed requireWorkspaceId, helper module under src/lib/enforcement/) is clean, no new dependencies, parameterized SQL throughout, and the 7 new tests give solid coverage. Two HIGH findings to address before merge:

HIGH

1. Fail-open bypass: requireAgentSelfAccess allows access when user.agent_id == null

File: src/lib/enforcement/workspace-scope.ts:72-80

When the URL segment is numeric and user.agent_id is null, the check is skipped entirely and return null (= allowed):

if (user.agent_id != null && user.agent_id !== targetNumeric) {
  return NextResponse.json(...403)
}
return null   // ← grants access when agent_id is null

Any auth path that resolves a User with agent_name set but agent_id === null (e.g. the _authResolverHook plugin hook in auth.ts:553-555, or any future helper that forgets to populate agent_id) can reach any other agent's heartbeat or memory by numeric id as long as it's in the same workspace. The test at the relevant line even documents this as "falls through to workspace filter" — but the workspace filter only ensures same-workspace, not same-agent.

Fix: fail-closed. Treat agent_id == null as deny when a numeric target is provided and the caller is an agent key:

// BAD
if (user.agent_id != null && user.agent_id !== targetNumeric) {
  return NextResponse.json(...403)
}
return null

// GOOD
if (user.agent_id == null || user.agent_id !== targetNumeric) {
  return NextResponse.json(...403)
}
return null

2. Behavioral breaking change: existing clients with missing workspace_id now get 400

File: All 7 modified route files

auth.user.workspace_id ?? 1 is replaced with requireWorkspaceId(auth.user) which returns 400 if workspace_id is falsy. For session auth this is fine — DB populates it with a default. But external integrations using _authResolverHook to inject a User without workspace_id will shift from "silently behaved as workspace 1" to "400 on every request" — without a deprecation period or migration note.

Fix: add a CHANGELOG entry calling out the breaking contract, and verify the plugin hook documentation requires workspace_id. Optionally, log a one-time warning on the first request that would have used the old ?? 1 fallback to help operators discover the gap.

MEDIUM

3. requireAgentTaskAccess blocks agent keys from unassigned tasks

File: src/lib/enforcement/workspace-scope.ts:101-109

When taskAssignedTo === null, the comparison taskAssignedTo !== user.agent_name is true, so it returns 403. An agent key cannot access a task in its own workspace that has assigned_to = null. Whether this is intended is unclear from the PR description.

Fix: clarify intent in a JSDoc comment. If agents should only see their assigned tasks, this is correct — say so. If they should also see unassigned ones, add taskAssignedTo === null as a pass-through.

4. requireAgentTaskAccess compares by agent name, not agent id

File: src/lib/enforcement/workspace-scope.ts:107

assigned_to is a freetext string in the DB. If two agents in different workspaces happen to share a name and the workspace filter ever leaks, this comparison would grant cross-access. Low exploitability today (workspace_id filter on the SQL query saves you), but it's a fragile design that depends on a layered defense.

Fix: long term, store assigned_to_agent_id alongside assigned_to. Short term, add a JSDoc note explicitly stating the workspace-id filter is the load-bearing defense.

LOW

• No test for requireAgentTaskAccess when taskAssignedTo is null (the case behind finding chore: remove internal artifacts #3).
• No test for POST /api/tasks/queue confirming an admin-scoped key can queue for a different agent (the positive bypass path).

Positive observations

• All 21 instances of ?? 1 fallback removed — no leftover unsafe defaults.
• 7 test files cover the missing-workspace_id (400) and cross-workspace overreach (403) paths.
• webhooks.ts fix correctly prevents delivery bookkeeping with a null workspace from silently writing workspace_id=1.
• Module boundaries are clean (helper isolated under src/lib/enforcement/).
• No hardcoded secrets, no SQL injection surface, no debug statements.

Summary

Severity	Count
HIGH	2
MEDIUM	2
LOW	2

Fix the agent_id null bypass and add the CHANGELOG/migration note for the workspace_id ?? 1 removal, then this is mergeable. Good work overall — the failure was the fail-open default in one branch of one check, not the architecture.

(Reviewer is read-only on this repo — flagging for a maintainer.)