Skip to content

feat(pentest): let customers add context to findings so retests are informed#3110

Merged
tofikwest merged 3 commits into
mainfrom
tofik/pentest-finding-context
Jun 11, 2026
Merged

feat(pentest): let customers add context to findings so retests are informed#3110
tofikwest merged 3 commits into
mainfrom
tofik/pentest-finding-context

Conversation

@tofikwest

@tofikwest tofikwest commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Why

Customers want to explain pentest findings — accepted-by-design rationale, remediation notes — so a retest validates against that context instead of blindly re-flagging the same issues. Direct customer ask (Slack):

"But how do i run retest on these issues with the additional context i provided instead of another 'blind' pentest?"

Key insight

Pentest findings are never stored in our DB (fetched live from Maced), and a re-run has zero memory of previous runs. But Maced's CreatePentestBody already supports an additionalContext free-text field that our API never exposed. This PR persists per-finding customer notes on our side and injects them into that field on every new scan of the same target.

What changed

DB (packages/db)

  • New SecurityPenetrationTestFindingContext model + migration: one note per org + provider issue id, unique on (organization_id, provider_issue_id), grouped by normalized target URL, with an issue_title snapshot so notes outlive provider run pruning.

API (apps/api)

  • New /v1/pentest-finding-contexts endpoints:
    • GET ?targetUrl= — list notes for a target (pentest:read)
    • PUT /:issueId — upsert a note (pentest:update); validates run ownership and that the issue belongs to the run; target URL taken from Maced, never from the client
    • DELETE /:issueId — remove a note (pentest:update)
  • createReport composes Maced's additionalContext from a new optional additionalContext DTO field plus all stored notes for the normalized target.
  • New pentest:update RBAC action (owner/admin; auditors stay read-only). This also makes pentest:update an available API-key scope so the MCP tools work for agents.
  • Follows the API endpoint contract: class DTOs with both decorator stacks, @ApiBody, summaries/descriptions within SEO limits, explicit MCP tool names (set-pentest-finding-context, delete-pentest-finding-context, list-pentest-finding-contexts).
  • Regenerated packages/docs/openapi.json (was stale on main — also picks up previously merged trust-portal/cloud-security endpoints).

App (apps/app)

  • Finding detail: permission-gated "Retest context" editor (RHF + zod); read-only roles see the saved note.
  • Create-run panel: optional "Context for the agent" textarea + hint showing how many saved notes for the target will be shared automatically.

Tests

  • API: 84/84 passing in the module — new specs for the pure util, contexts service (ownership/validation/upsert/delete), and controller incl. an RBAC-metadata assertion; existing service/billing specs extended for the new createReport flow.
  • App: 45/45 passing in the touched files — permission-gated FindingContextSection tests (update vs read-only) and CreateRunPanel payload tests (context included/omitted, auto-share hint).
  • Drive-by fix: both pentest controller specs previously failed to even load under jest (better-auth ESM import + Prisma client instantiation at import); fixed with permission.guard/@db mocks, keeping PERMISSIONS_KEY's real value so decorator metadata stays intact.
  • Pre-existing failures not touched by this PR: 11 stale tests in page.test.tsx/penetration-test-page-client.test.tsx (old "temporal UI" era design) fail identically on main.

Known limitation / follow-ups

  • If a retest re-flags an annotated issue, the new finding has a new issue id, so the note doesn't render on the new finding's detail view — it still reaches the agent's briefing. UI carry-over needs Maced's findingId cross-run semantics confirmed.
  • Maced's issues.update(status) triage API exists and is unused — candidate for a status-triage UI later.

🤖 Generated with Claude Code


Summary by cubic

Lets customers add context to pentest findings and runs so retests and downloaded reports are informed, not blind. Notes and optional run-level text are sent to Maced via additionalContext and appended to report markdown/PDF as a clearly attributed appendix; upserts now reject whitespace-only provider target URLs.

  • New Features

    • API: /v1/pentest-finding-contexts — GET by targetUrl (pentest:read), PUT/DELETE by issueId (pentest:update); validates run ownership/issue membership and rejects empty/whitespace normalized targets. Docs regenerated in packages/docs/openapi.json with MCP tools: set-pentest-finding-context, delete-pentest-finding-context, list-pentest-finding-contexts.
    • DB: new SecurityPenetrationTestFindingContext (unique per org+provider issue; grouped by normalized target URL with issue_title snapshot).
    • Runs & Reports: createReport composes provider additionalContext from the run’s optional additionalContext field plus saved notes for the target; report downloads append “Appendix: Customer context & management responses” to markdown/PDF (no notes -> passthrough; failures -> serve original).
    • App: permission-gated “Retest context” editor on finding detail; create-run panel adds optional “Context for the agent” with an auto-share hint. New pentest:update action enables editing (owner/admin, API-key scope).
  • Migration

    • Run database migrations to create the new table.
    • If using API keys/agents, grant the pentest:update scope to allow setting/removing notes.

Written for commit 1a45397. Summary will update on new commits.

Review in cubic

…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>
@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 Ready Ready Preview, Comment Jun 11, 2026 11:15pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
app Skipped Skipped Jun 11, 2026 11:15pm
portal Skipped Skipped Jun 11, 2026 11:15pm

Request Review

@mintlify

mintlify Bot commented Jun 11, 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 Jun 11, 2026, 10:21 PM

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

@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 29 files

Confidence score: 4/5

  • In apps/api/src/security-penetration-tests/pentest-finding-contexts.service.ts, provider target URLs that are only whitespace can pass validation and be persisted as an empty normalized target, which can create invalid finding context records and downstream confusion/filtering errors — trim and reject blank-after-trim URLs in validation 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/pentest-finding-contexts.service.ts Outdated
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>
@vercel vercel Bot temporarily deployed to Preview – portal June 11, 2026 23:00 Inactive
@vercel vercel Bot temporarily deployed to Preview – app June 11, 2026 23:00 Inactive
@tofikwest

Copy link
Copy Markdown
Contributor Author

Added in 5f59df7 — customer context now reaches the downloadable reports too.

The markdown/PDF reports come from the provider, so customer notes were invisible in the artifact customers hand to auditors. Both downloads now get a clearly attributed "Appendix: Customer context & management responses" — appended section in markdown, appended pages in the PDF (pdf-lib, same idioms as NdaPdfService).

Safety guarantees (the original provider report can never get worse):

  • zero notes → byte-identical passthrough (no re-save, no touch)
  • notes-lookup failure or unparseable provider PDF → logged, original bytes served unchanged
  • appendix is explicitly labeled as customer statements added after testing, with last-updated dates — never blended into the tester's conclusions
  • WinAnsi-safe text sanitization so arbitrary unicode in notes can't break PDF rendering

11 new tests (appendix rendering, pagination, unicode, corrupt-PDF fallback, service append/fallback paths); module suite at 95 passing.

🤖 Generated with Claude Code

…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>
@vercel vercel Bot temporarily deployed to Preview – portal June 11, 2026 23:13 Inactive
@vercel vercel Bot temporarily deployed to Preview – app June 11, 2026 23:13 Inactive
@tofikwest

Copy link
Copy Markdown
Contributor Author

Fixed the cubic finding in 1a45397upsert now validates the normalized target URL (normalizeTargetUrl(run.targetUrl)) instead of raw truthiness, so a whitespace-only provider URL gets a 502 instead of persisting a note with an empty target. Added a regression test (whitespace-only target → BAD_GATEWAY, no DB write). Module suite: 96 passing.

🤖 Generated with Claude Code

@tofikwest tofikwest merged commit 32e4a42 into main Jun 11, 2026
15 checks passed
@tofikwest tofikwest deleted the tofik/pentest-finding-context branch June 11, 2026 23:38
@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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants