Skip to content

fix(policies): rename CreateVersionDto to avoid swagger collision with automations - #3469

Merged
tofikwest merged 3 commits into
mainfrom
tofik/cs-761-bug-create-policy-version-mcp
Jul 22, 2026
Merged

tofikwest merged 3 commits into
mainfrom
tofik/cs-761-bug-create-policy-version-mcp

Conversation

@tofikwest

@tofikwest tofikwest commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Problem

The create-policy-version MCP tool is completely broken. Callers either hit a 400 error from the API ("property scriptKey should not exist", "property version should not exist") or are blocked by the MCP client schema validation before the call even goes out. There's no valid path to use this tool.

Root cause

Two DTO classes share the name CreateVersionDto: one in the policies module (with sourceVersionId and changelog) and one in the automations module (with version and scriptKey). NestJS Swagger keys schema components by class name, so they collide in the OpenAPI spec. The merged component ended up with version+scriptKey marked as required. Speakeasy then generated the MCP tool from that collision, making those fields required. But the live policies endpoint validates inbound DTOs with a whitelist that rejects those two fields, causing the 400.

Fix

Gave the policies DTO a distinct OpenAPI component name via @ApiSchema({ name: 'CreatePolicyVersionDto' }). The class itself stays CreateVersionDto — only its Swagger component name changes — which is enough to remove the name collision with the automations DTO, so the policies endpoint gets its own correct schema (optional sourceVersionId + changelog).

Also regenerated packages/docs/openapi.json, the artifact Speakeasy builds the MCP server from, so the corrected schema actually reaches the generated create-policy-version tool. The regen is additive only: it introduces CreatePolicyVersionDto, repoints the policies versions endpoint to it, and catches the spec up to already-merged endpoints whose PRs never regenerated it (ISMS bulk measurements, four trust-portal request bodies, extra optional ISMS register fields). No existing component shape changed and nothing was removed.

Explicitly NOT touched

No changes to endpoint handlers, validation pipe config, auth, RBAC, billing, or automations module. The automations CreateVersionDto remains as-is.

Verification

✅ Unit tests for policies version creation pass locally with the distinct OpenAPI component name
✅ Added regression test asserting that the policy version endpoint accepts sourceVersionId and changelog and rejects version/scriptKey in the request body

Fixes CS-761


Summary by cubic

Fixes the create-policy-version MCP tool by renaming the policies DTO to avoid an OpenAPI component collision and regenerating the spec so the tool uses the correct schema. Restores the proper request shape (sourceVersionId, changelog) and removes 400s and client-side schema blocking. Addresses CS-761.

  • Bug Fixes
    • Assigned a unique Swagger name via @ApiSchema({ name: 'CreatePolicyVersionDto' }) to stop @nestjs/swagger component collision with automations’ CreateVersionDto.
    • Regenerated packages/docs/openapi.json so the MCP tool is built from CreatePolicyVersionDto; also pulls in additive spec updates (ISMS bulk measurements, trust-portal request bodies, extra ISMS fields).
    • Added OpenAPI tests to ensure distinct components and verify policies only expose sourceVersionId and changelog (no version/scriptKey).

Written for commit c303de2. Summary will update on new commits.

Review in cubic

…h automations

## Problem
The create-policy-version MCP tool is completely broken. Callers either hit a 400 error from the API ("property scriptKey should not exist", "property version should not exist") or are blocked by the MCP client schema validation before the call even goes out. There's no valid path to use this tool.

## Root cause
Two DTO classes share the name CreateVersionDto: one in the policies module (with sourceVersionId and changelog) and one in the automations module (with version and scriptKey). NestJS Swagger keys schema components by class name, so they collide in the OpenAPI spec. The merged component ended up with version+scriptKey marked as required. Speakeasy then generated the MCP tool from that collision, making those fields required. But the live policies endpoint validates inbound DTOs with a whitelist that rejects those two fields, causing the 400.

## Fix
Renamed the policies DTO class from CreateVersionDto to CreatePolicyVersionDto to eliminate the name collision in Swagger. This allows the correct schema (sourceVersionId and changelog) to be generated for the policies endpoint and for MCP tool regeneration.

## Explicitly NOT touched
No changes to endpoint handlers, validation pipe config, auth, RBAC, billing, or automations module. The automations CreateVersionDto remains as-is.

## Verification
✅ Unit tests for policies version creation pass locally with the renamed DTO
✅ Added regression test asserting that the policy version endpoint accepts sourceVersionId and changelog and rejects version/scriptKey in the request body
@linear

linear Bot commented Jul 21, 2026

Copy link
Copy Markdown

CS-761

@vercel

vercel Bot commented Jul 21, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
app Ready Ready Preview, Comment Jul 22, 2026 2:52am
comp-framework-editor Ready Ready Preview, Comment Jul 22, 2026 2:52am
portal Ready Ready Preview, Comment Jul 22, 2026 2:52am

Request Review

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

cubic analysis

No issues found across 2 files

Confidence score: 5/5

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

Linked issue analysis

Linked issue: CS-761: [Bug] - create-policy-version MCP Tool Schema Out of Sync with comp.ai API

Status Acceptance criteria Notes
✅ Eliminate OpenAPI component name collision so the policies DTO is distinct from automations' CreateVersionDto The policies DTO was given an explicit OpenAPI name and the new OpenAPI test asserts the policy and automation request-body components are distinct.
✅ OpenAPI schema for POST /v1/policies/{id}/versions exposes sourceVersionId and changelog and does not include/require version or scriptKey The OpenAPI-level test verifies the properties and required arrays to ensure the policies shape contains sourceVersionId/changelog and lacks version/scriptKey.
⚠️ create-policy-version MCP tool is unblocked end-to-end (no client-side schema blocking or API 400s due to scriptKey/version) Server-side DTO rename and OpenAPI tests fix the root cause that produced client/schema mismatches; however the PR does not include a regenerated MCP client or an end-to-end call demonstrating the tool now succeeds, so full end-to-end evidence is not present in the diff.

Re-trigger cubic

Regenerate the committed OpenAPI spec so the create-policy-version MCP tool
is generated from the corrected schema (CreatePolicyVersionDto with optional
sourceVersionId + changelog) instead of the collided CreateVersionDto. The
Speakeasy MCP server builds from packages/docs/openapi.json, so the DTO fix
only reaches the published tool once this artifact is regenerated.

Also catches the spec up to already-merged endpoints whose PRs did not
regenerate it: the ISMS bulk-measurements endpoint, four trust-portal request
bodies, and extra optional fields on two ISMS register endpoints. All changes
are additive — no existing component shape changed and nothing was removed.
@mintlify

mintlify Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
CompAI 🟢 Ready View Preview Jul 21, 2026, 10:44 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@tofikwest
tofikwest merged commit 2d5290a into main Jul 22, 2026
15 checks passed
@tofikwest
tofikwest deleted the tofik/cs-761-bug-create-policy-version-mcp branch July 22, 2026 02:55
Marfuen added a commit that referenced this pull request Jul 22, 2026
Regenerated from the merged code so the spec carries #3469's
CreatePolicyVersionDto rename plus the current ISMS-audit schema (main's
committed spec had drifted). Written as-generated by the dev boot.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Marfuen added a commit that referenced this pull request Jul 22, 2026
…org owner (#3472)

* fix(auth): attribute API-key mutations to the key's creator, not the org owner

API keys were org-scoped with no recorded creator, so ActingUserResolver
attributed every API-key/MCP mutation to the org's oldest owner. In the audit
trail this made all automation look like the owner performed it, masking the
real actor — a problem for a compliance product.

- Add ApiKey.createdByMemberId (nullable, FK to Member, onDelete: SetNull).
  Populated on creation with the acting member. Nullable so legacy keys and
  keys whose creator was removed fall back cleanly.
- create-api-key now records the creating member; validateApiKey surfaces it.
- HybridAuthGuard puts it on the request; ActingUserResolver attributes the
  mutation to the creator (when still an active member of the org), else falls
  back to the org owner as before.
- Tests for both the creator-attribution path and the deactivated-creator
  fallback.

No backfill: existing keys have no recorded creator and keep falling back to
the org owner.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(auth): select createdByMemberId in the legacy API-key lookup path

The legacy-key branch's select was missing createdByMemberId (its deeper
indentation meant the earlier bulk edit didn't cover it), so the return that
references legacyMatch.createdByMemberId failed to typecheck. Add the field to
the legacy select to match the primary path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(audit): resolve acting user for API-key mutations so they're logged

The global AuditLogInterceptor early-returned when request.userId was
absent, so API-key (and MCP) mutations were never audit-logged at all —
the createdByMemberId attribution added earlier had no effect on the
trail. Inject ActingUserResolver and resolve the responsible user (key
creator, else org owner) when there's no session userId; skip logging
only when no user can be attributed (no null-FK rows).

Also fixes a pre-existing bug surfaced once the spec could load: the
control mapping/unmapping descriptions read "policie" because the
resolver naive-stripped the trailing "s" of "policies". Use the known
Prisma model name (policies→policy) with an "ies"→"y" fallback.

The interceptor spec never ran before (it pulled better-auth's ESM
subpaths via permission.guard); mock @trycompai/auth like the other
specs so all 41 tests execute.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(auth): attribute API-key mutations to the acting user across silent sinks

The interceptor fix covered automatic @RequirePermission audit logging. This
covers the remaining sinks where an API-key mutation succeeded but attribution
was silently lost or credited to the org owner instead of the responsible user:

- vendors create + triggerAssessment: the controller now resolves the acting
  user and threads it as createdByUserId, so the auto-generated risk-assessment
  task ("created this task") credits the key creator, not the admin fallback.
- cloud-security scan: attribute scan_completed via ActingUserResolver instead
  of raw req.userId (was skipped entirely for API keys).
- policies publish-all: per-policy audit rows were dropped for API-key auth
  (authContext.userId undefined) — resolve the actor first.
- isms createRow: enteredById (a Member FK) was null for API keys — resolve the
  acting member.

ActingUserResolver now populates memberId on every path (session member, key
creator, or fallback owner's member), so Member-FK sinks like isms enteredById
attribute correctly. Owner lookup selects the member id alongside the user id.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(auth): address cubic review — actor attribution edge cases

- vendors create/triggerAssessment: 400 when no actor resolves (org has no
  owner) instead of creating a vendor whose assessment task has no attributed
  user — matches ActingUserResolver's contract.
- cloud-security scan: attribution is best-effort (try/catch) so a transient
  resolver/audit failure can't fail an already-completed scan and invite a
  re-run.
- isms bulkCreateMeasurements: resolve the acting member (session-first, then
  api-key creator/owner) so bulk saves via API key don't persist null enteredById.
- hybrid-auth guard: service-token x-user-id now sets request.memberId so
  Member-FK sinks can attribute service-token-acting mutations.
- tests: createApiKey creator attribution (session forwards memberId, api-key
  forwards null), vendors 400-on-null, isms bulk api-key attribution.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(auth): restrict service-token x-user-id to active members

Cubic follow-up: the x-user-id member lookup didn't filter deactivated /
inactive memberships, so an offboarded user supplied via x-user-id could
receive new audit / enteredById attribution. Add deactivated:false + isActive:true
to the lookup (matching ActingUserResolver's filters); an inactive member now
resolves to no acting user and falls back to owner resolution downstream.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(auth): cover service-token active-membership restriction

Cubic follow-up: add regression coverage for the x-user-id acting-member
resolution — an active member populates request.userId + memberId (and the
lookup is scoped to deactivated:false/isActive:true), while a deactivated/
inactive member sets neither, so the restriction can't silently regress.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(audit): record caller provenance for non-session attributions

Security review follow-up (non-repudiation): API-key / service-token
mutations that resolve to the org owner (or key creator) were recorded in
the audit trail with no marker, so they read as if the user personally acted
in a session. Carry ActingUserResolver's callerLabel through the interceptor
and cloud-security scan — append `[via API key "..."]` to the description and
store `via` in the audit data JSON. Mirrors the existing exception /
scan-mode endpoints. Session actions are unaffected (no label).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(security): add security-review skill, agent, hook, and pre-push gate

Adds a first-party security-review skill + security-reviewer agent covering the
high-risk vuln classes (broken access control, tenant isolation, injection,
secrets, SSRF, auth/session, unsafe uploads, mass assignment, IDOR). Makes it
run automatically via a behavior-based PostToolUse nudge (API surface, services,
route handlers, trigger jobs, by-id data access, plus content patterns for XSS/
raw SQL/exec), inclusion in production-readiness, and a Husky pre-push hard gate
(bypass after review: SECURITY_REVIEWED=1 git push).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(security): auto-run security review before push (PreToolUse agent hook)

Adds a PreToolUse agent hook on `git push` that automatically runs the
security-reviewer on the outgoing diff (origin/main...HEAD) and blocks the push
if it finds an unaddressed P1/P2 — so the security skill triggers automatically
on push instead of relying on a manual run. Fires for Claude-driven pushes; the
Husky pre-push remains the backstop for terminal/CI pushes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(security): add Claude security-review GitHub Action; drop local push block

Adopts Anthropic's official claude-code-security-review Action as the
authoritative gate — runs on every PR, diff-scoped, posts inline findings —
layered with repo-specific instructions (tenant isolation, authz-vs-attribution,
IDOR). Removes the local push friction that doesn't actually run a review: the
Husky pre-push hard block and the PostToolUse reminder. The security actually
runs in two places now: the PreToolUse agent hook (Claude-driven pushes) and the
PR Action (all PRs). Prereq: add an ANTHROPIC_API_KEY repo secret.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(security): remove misfiring PreToolUse security hook

The agent hook fired on non-git-push commands and blocked them (its
allow/block semantics were inverted and the `if` filter didn't scope). The
PR GitHub Action is the reliable auto-run gate; drop the local hook.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(security): review every commit on a PR, not just the first

Set run-every-commit so the security Action re-reviews the latest diff on each
push to a PR. Without it the action runs once per PR and skips later commits,
leaving code added after the first review unchecked while the required check
stays green. Findings remain advisory PR comments (no merge block on noise).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(api): regenerate openapi.json after merging main

Regenerated from the merged code so the spec carries #3469's
CreatePolicyVersionDto rename plus the current ISMS-audit schema (main's
committed spec had drifted). Written as-generated by the dev boot.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
claudfuen pushed a commit that referenced this pull request Jul 22, 2026
# [3.106.0](v3.105.0...v3.106.0) (2026-07-22)

### Bug Fixes

* **auth:** attribute API-key mutations to the key's creator, not the org owner ([#3472](#3472)) ([206ed96](206ed96)), closes [hi#risk](https://github.com/hi/issues/risk)
* **deps:** bump adm-zip 0.5.18 -> 0.6.0 in apps/api (Dependabot [#88](https://github.com/trycompai/comp/issues/88)/[#89](https://github.com/trycompai/comp/issues/89)) ([#3462](#3462)) ([300f2a1](300f2a1)), closes [#3451](#3451)
* **deps:** override tar to ^7.5.19 to clear node-tar Dependabot alerts ([#94](https://github.com/trycompai/comp/issues/94)-[#104](https://github.com/trycompai/comp/issues/104)) ([#3466](#3466)) ([8ab5709](8ab5709))
* **deps:** patch engine.io ([#93](#93)) and body-parser ([#92](#92)) Dependabot alerts ([#3464](#3464)) ([94c33b1](94c33b1))
* **isms:** harden internal-audit validation and edge cases from deploy review ([#3473](#3473)) ([c6c7379](c6c7379))
* **policies:** create draft version on policy regenerate instead of overwriting published ([#3471](#3471)) ([ff31dbd](ff31dbd))
* **policies:** delete detached PDF objects when regenerating a draft ([#3474](#3474)) ([ecd1bd0](ecd1bd0))
* **policies:** rename CreateVersionDto to avoid swagger collision with automations ([#3469](#3469)) ([2d5290a](2d5290a))

### Features

* **isms:** internal audit programme, plan and report — clause 9.2 (CS-724) ([#3468](#3468)) ([42e5ebd](42e5ebd)), closes [hi#impact](https://github.com/hi/issues/impact)
@claudfuen

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 3.106.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

This branch was successfully deployed

4 active (1 outdated) deployments
Preview – comp-framework-editor — c303de2d Deployed Jul 22, 2026 by vercel[bot]
Preview – app — c303de2d Deployed Jul 22, 2026 by vercel[bot]
Preview – portal — c303de2d Deployed Jul 22, 2026 by vercel[bot]
staging - packages/docs — d73c0da5 Deployed Jul 21, 2026 by mintlify[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants