Skip to content

[comp] Production Deploy#3111

Merged
tofikwest merged 10 commits into
releasefrom
main
Jun 12, 2026
Merged

[comp] Production Deploy#3111
tofikwest merged 10 commits into
releasefrom
main

Conversation

@github-actions

@github-actions github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

This is an automated pull request to release the candidate branch into production, which will trigger a deployment.
It was created by the [Production PR] action.


Summary by cubic

Adds customer context to pentest findings and run-level context. Retests include saved notes in the provider’s additionalContext, and report downloads append a “Customer context & management responses” appendix in Markdown and PDF.

  • New Features

    • API: new /v1/pentest-finding-contexts endpoints
      • GET by targetUrl (requires pentest:read)
      • PUT/DELETE per finding (requires pentest:update), with normalized target validation
    • Create run accepts optional additionalContext (max 4000). Service composes provider additionalContext from this plus saved notes for the same normalized target URL; lookup is best‑effort and falls back to caller context on DB errors. Composed briefing is capped at 20,000 chars — drops whole notes with an “(N more notes omitted for length)” marker; user‑typed context is always kept.
    • App: “Retest context” editor on Finding Detail and run-level context field with auto-append hint. Permission-gated to users with pentest:update.
    • Reports: append a clearly attributed customer-context appendix to Markdown and PDF. No notes → passthrough; any failure → serve original artifact unchanged. PDF wrapper now hard-breaks long tokens; URL normalizer strips trailing slashes from the path but preserves query values.
    • Data model: new SecurityPenetrationTestFindingContext table with indexes; normalized target URL matching. OpenAPI updated.
  • Migration

    • Run Prisma migrations.
    • Ensure roles include pentest:update; owners/admins already granted in packages/auth.
    • API is backward compatible; clients may optionally send additionalContext when creating runs.

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

Review in cubic

github-actions Bot and others added 5 commits June 11, 2026 21:11
…nformed

Customers asked to explain findings (accepted-by-design rationale,
remediation notes) so a retest doesn't blindly re-flag them
(CS request: 'how do i run retest with the additional context i
provided instead of another blind pentest').

- add SecurityPenetrationTestFindingContext table (+migration): one
  note per org+provider issue, grouped by normalized target URL
- new /v1/pentest-finding-contexts endpoints: GET list by target
  (pentest:read), PUT/DELETE per finding (new pentest:update action,
  granted to owner/admin); MCP tool names + OpenAPI metadata included
- createReport now composes Maced's additionalContext from the new
  optional additionalContext DTO field plus all saved notes for the
  target, so every re-run is an informed retest
- app: permission-gated 'Retest context' editor on FindingDetail,
  optional run-level context field + auto-share hint on CreateRunPanel
- regenerate packages/docs/openapi.json
- tests: util/service/controller specs (api), FindingContextSection +
  CreateRunPanel payload tests (app); fix pre-existing controller spec
  load failures by mocking permission.guard/@db

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The markdown and PDF reports come from the provider and never showed
the customer's finding-context notes — the artifact handed to auditors
lost exactly the justifications the notes exist for.

- report-appendix.util.ts: render notes as a clearly attributed
  'Appendix: Customer context & management responses' — appended
  section for markdown, appended pages (pdf-lib, NdaPdfService idioms)
  for PDF; WinAnsi-safe sanitization, word wrap, pagination
- getReportOutput/getReportPdf attach the appendix for the run's
  target; zero notes -> byte-identical passthrough; notes-lookup or
  PDF-parse failures are logged and the original provider artifact is
  served unchanged (appendix is additive, never load-bearing)
- tests: 11 new (util markdown/pdf/pagination/unicode/corrupt-input +
  service append/fallback paths), module suite now 95 passing

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…upsert

Validate the normalized target instead of the raw string so a
whitespace-only provider URL can't persist a note with an empty
target that no future scan lookup could match (cubic review).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feat(pentest): let customers add context to findings so retests are informed
@vercel

vercel Bot commented Jun 11, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
comp-framework-editor (staging) Ready Ready Preview, Comment Jun 12, 2026 12:22am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
app (staging) Skipped Skipped Jun 12, 2026 12:22am
portal (staging) Skipped Skipped Jun 12, 2026 12:22am

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.

3 issues found across 31 files

Confidence score: 3/5

  • In apps/api/src/security-penetration-tests/security-penetration-tests.service.ts, the finding-context lookup can fail closed and block pentest creation if the DB call errors, which risks user-facing request failures during transient outages — make this lookup best-effort (log and continue) before merging.
  • In apps/api/src/security-penetration-tests/finding-context.util.ts, trimming trailing / on the full URL can alter query parameter values and create incorrect finding-context keys, causing mismatches or duplicate context records — normalize only the path portion (or use URL parsing) and add a regression test before merging.
  • In apps/api/src/security-penetration-tests/report-appendix.util.ts, long unbroken tokens are not wrapped in generated PDFs, so notes with long URLs/IDs can overflow margins and become unreadable for customers — add hard-wrap behavior for overlong tokens (with a fixture test) before merging.

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread apps/api/src/security-penetration-tests/report-appendix.util.ts
Comment thread apps/api/src/security-penetration-tests/finding-context.util.ts Outdated
- createReport notes lookup is now best-effort via the shared quiet
  helper: a DB failure (transient outage or table missing mid-deploy)
  logs and proceeds with the caller's own context instead of blocking
  pentest creation
- normalizeTargetUrl strips trailing slashes from the path only, so a
  '/' at the end of a query value is preserved instead of corrupting
  the finding-context key
- PDF appendix wrapper hard-breaks unbroken tokens (long URLs/IDs)
  that exceed the line width instead of overflowing the margin

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@tofikwest

Copy link
Copy Markdown
Contributor

The three cubic findings on the pentest finding-context feature are fixed in #3112 (targeting main, so it'll flow into this deploy once merged):

  1. createReport notes lookup is now best-effort — a DB failure (incl. the table missing mid-deploy before the migration runs) logs and proceeds with the caller's own context instead of blocking pentest creation.
  2. normalizeTargetUrl strips trailing slashes from the path only — query values like ?next=/portal/ are preserved; origin/path-only targets normalize exactly as before so existing keys are unaffected.
  3. The PDF appendix wrapper hard-breaks overlong unbroken tokens (long URLs/IDs) at character level so they can't overflow the margin.

All three have regression tests; module suite at 100 passing.

🤖 Generated with Claude Code

…fixes

fix(pentest): address cubic review findings on finding-context flow
@tofikwest

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review it

@cubic-dev-ai

cubic-dev-ai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review it

@tofikwest I have started the AI code review. It will take a few minutes to complete.

@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.

1 issue found across 31 files

Confidence score: 4/5

  • In apps/api/src/security-penetration-tests/finding-context.util.ts, the composed additionalContext can grow past the 4000-character contract after stored notes are appended, which could cause downstream validation/rejection or truncated context in consumers; add a hard length cap (and ideally a test for over-limit note sets) before merging to de-risk this path.

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread apps/api/src/security-penetration-tests/finding-context.util.ts Outdated
The provider has no documented limit (verified against its OpenAPI spec
and prompt-injection source), but the composed briefing (user context +
all stored notes for a target) was unbounded. Cap at 20,000 chars by
dropping whole notes — never cutting one mid-sentence — with an explicit
'(N more notes omitted for length)' marker so the agent knows the list
is incomplete. User-typed context is always kept (DTO-capped at 4000).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@tofikwest

Copy link
Copy Markdown
Contributor

Re the latest cubic finding (unbounded additionalContext): partially accurate, fixed in #3113.

  • Wrong premise: there is no "4000-character contract" — verified against Maced's live OpenAPI spec (no maxLength on additionalContext) and their server source (no cap anywhere from API to agent prompt). The 4000 limit is our own DTO validation on the user-typed field only, so nothing downstream rejects or truncates today.
  • Right instinct: the composed briefing (user text + all stored notes for a target) was unbounded. fix(pentest): cap the composed additionalContext briefing at 20k chars #3113 caps it at 20k chars, dropping whole notes with an explicit (N more notes omitted for length) marker — no silent truncation, user context always kept, and the auditor-facing report appendix intentionally uncapped.

🤖 Generated with Claude Code

@tofikwest tofikwest merged commit 425d112 into release Jun 12, 2026
13 checks passed
@claudfuen

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 3.79.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants