Skip to content

feat(api): unblock cloud-tests mutations for API key + service token callers#2883

Merged
tofikwest merged 1 commit into
mainfrom
tofik/api-key-cloud-tests-mutations
May 20, 2026
Merged

feat(api): unblock cloud-tests mutations for API key + service token callers#2883
tofikwest merged 1 commit into
mainfrom
tofik/api-key-cloud-tests-mutations

Conversation

@tofikwest
Copy link
Copy Markdown
Contributor

@tofikwest tofikwest commented May 20, 2026

Summary

Customer Josh hit a 401 calling POST /v1/cloud-security/findings/:id/exception with an X-API-Key from his automation pipeline. The same restriction existed on revokeException and updateAwsScanMode. With MCP integration coming next, we want the full cloud-tests mutation surface to be callable via API key + service token, not session only.

This PR removes the inline session-only wall on all three endpoints by introducing a shared ActingUserResolver that attributes API key / service token mutations to the org's oldest owner — keeping the audit trail intact without forcing callers to manage user IDs themselves. The audit log description prepends [via API key "<name>"] so reviewers see at a glance that it was automation.

What's preserved (regression safety)

  • Authentication unchanged — HybridAuthGuard runs as before. Invalid keys still rejected.
  • RBAC unchanged — PermissionGuard runs as before. Caller still needs integration:update scope.
  • Cross-tenant isolation preserved — owner lookup scoped to the calling org via Member.organizationId filter.
  • Session callers: ZERO behavior change. The resolver short-circuits to req.userId with no DB query, and the audit description is exactly the same as today (no [via ...] prefix).
  • Other write endpoints untouched — vendor:create, risk:create, evidence:create work exactly as before.

What changes

Caller Before After
Session ✅ Works (no audit prefix) ✅ Works (no audit prefix) — IDENTICAL behavior
Service token + x-user-id ✅ Works (no audit prefix) ✅ Works (no audit prefix) — IDENTICAL behavior
Service token without x-user-id ❌ 401 ✅ Works, attributed to org owner, audit description prefixed [via service "X"]
API key ❌ 401 ✅ Works, attributed to org owner, audit description prefixed [via API key "X"]
API key, org has no owner-role member ❌ 401 ❌ 400 with actionable message ("ensure your org has an owner")

Files

New (4):

  • apps/api/src/auth/acting-user.service.ts — shared resolver, single source of truth for "who do I attribute this mutation to?"
  • apps/api/src/auth/acting-user.service.spec.ts — 10 tests
  • apps/api/src/cloud-security/aws-scan-mode.service.spec.ts — 5 tests (none existed)
  • apps/api/src/cloud-security/cloud-security.controller.spec.ts — 9 tests (none existed for these endpoints)

Modified (8):

  • apps/api/src/auth/auth.module.ts — register ActingUserResolver
  • apps/api/src/auth/api-key.service.tsApiKeyValidationResult now includes apiKeyId, apiKeyName
  • apps/api/src/auth/hybrid-auth.guard.ts — populate request.apiKeyId + request.apiKeyName
  • apps/api/src/auth/types.ts — type additions
  • apps/api/src/cloud-security/cloud-security.controller.ts — replace 3 inline 401 walls with resolver calls; better OpenAPI descriptions
  • apps/api/src/cloud-security/exception.service.ts — accept callerLabel, use it in audit description
  • apps/api/src/cloud-security/aws-scan-mode.service.ts — accept callerLabel, use it in audit description
  • apps/api/src/cloud-security/exception.service.spec.ts — +2 tests for callerLabel propagation

Tests

253 cloud-security + auth tests pass. Three pre-existing failures unchanged (same suites fail on main — Postgres TLS env issues, unrelated to this PR).

Key regression guards added:

  • Session caller MUST NOT hit the DB for owner lookup (perf regression guard)
  • Cross-tenant: owner lookup scoped to calling org (security regression guard)
  • Oldest-owner pick is deterministic (orderBy: { createdAt: 'asc' }) — stable attribution across owner add/remove
  • No-owner case returns 400 NOT 500 (Prisma FK error would otherwise leak as 500)

Reply to send the customer after merge

"Fixed. Your call works as-is now — no header changes needed. The exception is attributed to your org's owner, and the audit log records that it came from your API key ''. If you want finer-grained attribution per call, pass X-User-Id with a valid user ID from your org and we'll use that instead."

Test plan

  • 10 acting-user.service tests pass
  • 12 exception.service tests pass (+2 callerLabel)
  • 5 aws-scan-mode.service tests pass (NEW)
  • 9 cloud-security.controller tests pass (NEW)
  • 253 cloud-security + auth tests pass overall
  • Typecheck clean for changed files
  • Manual: hit POST /v1/cloud-security/findings/:id/exception with X-API-Key + valid body → expect 201, exception created with markedById = <owner user>, audit log contains [via API key "<key name>"]
  • Manual: same call against an org with no owner-role member → expect 400 with actionable message
  • Manual: same call from UI (session) → audit description unchanged (no [via ...] prefix), behavior identical to today

🤖 Generated with Claude Code


Summary by cubic

Unblocked cloud-security mutations for API keys and service tokens by attributing actions to the org’s oldest owner via a new ActingUserResolver, with clear audit labels. Removes the session-only wall on exception create/revoke and AWS scan mode updates while keeping auth and RBAC intact.

  • New Features
    • Unblocks exception create/revoke and AWS scan mode mutations for API keys and service tokens.
    • Adds ActingUserResolver for attribution: short-circuits for session or X-User-Id; otherwise falls back to the oldest org owner; prefixes audit text with [via API key "<name>"] or [via service "<name>"]; returns 400 if no owner.
    • Updates controllers/services to pass an optional callerLabel so audit logs show automation; session behavior is unchanged and does not query the DB.
    • Exposes apiKeyId and apiKeyName on the request via api-key.service and hybrid-auth.guard for accurate audit context.

Written for commit 26e53da. Summary will update on new commits. Review in cubic

…callers

A customer (Josh) hit a 401 calling
POST /v1/cloud-security/findings/:id/exception with an X-API-Key:
"Marking an exception requires session authentication." The same
inline check existed on revokeException and updateAwsScanMode. With
MCP integration coming soon we want the full cloud-tests mutation
surface to be API-callable.

The reason the wall existed: AuditLog.userId + FindingException.markedById
are both String/FK to User, and API keys are org-scoped (no per-user
identity). This change introduces a shared ActingUserResolver that
attributes API key / service-token mutations to the org's oldest
owner-role member — same pattern 19+ other services in the codebase
already use for owner lookups. The audit log description prepends
`[via API key "<name>"]` so reviewers see at a glance that it was
automation, not a UI action.

What's preserved:
  - Authentication unchanged (HybridAuthGuard runs as before).
  - RBAC unchanged (PermissionGuard runs as before — caller still
    needs integration:update scope).
  - Cross-tenant isolation preserved (owner lookup scoped to the
    calling org).
  - Session callers: zero behavior change — resolver short-circuits
    to req.userId with NO DB query. Audit description unchanged
    (no [via ...] prefix).
  - Existing API key writes (vendor:create, risk:create, etc.):
    untouched.
  - Service tokens passing x-user-id: unchanged (HybridAuthGuard
    already sets req.userId, resolver short-circuits).

What changes:
  - Service tokens without x-user-id: now succeed with owner-fallback
    attribution (previously hit the inline 401 too).
  - API key callers: now succeed with owner-fallback attribution.
  - Org with no owner-role member: 400 with actionable message
    ("ensure your organization has at least one user with the owner
    role"). Soft failure, never a 500.

Tests:
  - 10 acting-user.service tests (session short-circuit, owner
    fallback, cross-tenant scoping, deterministic oldest-owner pick,
    no-owner soft failure, caller-label formatting)
  - 12 exception.service tests (+2 for callerLabel propagation —
    audit description prefix + metadata field)
  - 5 NEW aws-scan-mode.service tests (idempotent no-op, AWS-only
    enforcement, owner-fallback callerLabel)
  - 9 NEW controller-level tests covering all 3 endpoints × 3
    scenarios (session, API-key+owner, no-owner → 400)
  - 253 cloud-security + auth tests pass (3 pre-existing TLS-env
    failures unrelated — same suites fail on main)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented May 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
comp-framework-editor Ready Ready Preview, Comment May 20, 2026 7:36pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
app Skipped Skipped May 20, 2026 7:36pm
portal Skipped Skipped May 20, 2026 7:36pm

Request Review

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 12 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@tofikwest tofikwest merged commit e2ed683 into main May 20, 2026
11 checks passed
@tofikwest tofikwest deleted the tofik/api-key-cloud-tests-mutations branch May 20, 2026 19:58
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