From 24353c4d8b9d4f00b8e7df33fefe3327433acf65 Mon Sep 17 00:00:00 2001
From: Tofik Hasanov <annexcies@gmail.com>
Date: Tue, 23 Jun 2026 15:10:39 -0400
Subject: [PATCH] fix(pentest): use run id instead of issue run id for context
 submission
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Problem

Users see a bare HTTP 404 error when submitting context or remediation notes on a penetration test finding.

## Root cause

FindingContextSection.tsx computes the runId with wrong precedence: `resolvedRunId = issue.runId ?? runId`. Since Issue.runId is non-nullable in the generated types, it always wins. This causes the save endpoint to send the provider's per-issue runId instead of the actual pentest run.id.

The ownership check in assertRunOwnership looks for an ownership marker keyed on providerRunId (which equals run.id), so it finds nothing and throws a NotFoundException with no message field. This surfaces to the user as a bare "HTTP 404:" error.

Viewing a finding uses run.id and works fine. Saving uses issue.runId and 404s. Code-path asymmetry.

## Fix

Flip the precedence to `runId ?? issue.runId`. This ensures we use the pentest run id first, falling back to the issue's run id only if needed.

## Explicitly NOT touched

- No changes to assertRunOwnership logic or error handling
- No database migrations or schema changes
- No changes to the ownership marker creation

## Verification

✅ Tested context submission on a pentest finding in org_69840a18d99f827ad3de64c8
✅ Ownership check now finds the correct marker
✅ Save endpoint returns 200 instead of 404
✅ Finding view still works as before
---
 .../FindingContextSection.test.tsx            | 33 +++++++++++++++++++
 .../_components/FindingContextSection.tsx     |  8 ++++-
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingContextSection.test.tsx b/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingContextSection.test.tsx
index e6792dc26..9e72c6eef 100644
--- a/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingContextSection.test.tsx
+++ b/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingContextSection.test.tsx
@@ -125,6 +125,39 @@ describe('FindingContextSection', () => {
     });
   });
 
+  it('saves against the page run id, not the provider per-issue runId', async () => {
+    // Maced's Issue.runId is a per-issue run id that has no ownership-marker
+    // row, so sending it makes the API's upsert → getReport → assertRunOwnership
+    // 404 (the bare "HTTP 404:" toast). The PUT must carry the pentest run id
+    // the page is built on.
+    permissionsMock.canUpdatePentest = true;
+    contextsMock.saveContext.mockResolvedValue(undefined);
+    const user = userEvent.setup();
+
+    render(
+      <FindingContextSection
+        orgId="org_1"
+        issue={{ ...issue, runId: 'run_maced_per_issue' }}
+        runId="run_page"
+        targetUrl="https://app.example.com"
+      />,
+    );
+
+    await user.type(
+      screen.getByRole('textbox'),
+      'Accepted by design — reads are non-sensitive.',
+    );
+    await user.click(screen.getByRole('button', { name: /save context/i }));
+
+    await waitFor(() => {
+      expect(contextsMock.saveContext).toHaveBeenCalledWith({
+        issueId: 'issue_1',
+        runId: 'run_page',
+        context: 'Accepted by design — reads are non-sensitive.',
+      });
+    });
+  });
+
   it('lets pentest:update users remove a saved note', async () => {
     permissionsMock.canUpdatePentest = true;
     contextsMock.contextByIssueId = new Map([['issue_1', savedNote]]);
diff --git a/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingContextSection.tsx b/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingContextSection.tsx
index 225657776..b7d206de7 100644
--- a/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingContextSection.tsx
+++ b/apps/app/src/app/(app)/[orgId]/security/penetration-tests/_components/FindingContextSection.tsx
@@ -44,7 +44,13 @@ export function FindingContextSection({
   const { contextByIssueId, isSaving, saveContext, removeContext } =
     usePentestFindingContexts(orgId, targetUrl);
   const existing = contextByIssueId.get(issue.id);
-  const resolvedRunId = issue.runId ?? runId ?? null;
+  // Save against the pentest run id the page is built on (run.id), NOT
+  // issue.runId. Maced's Issue.runId is a per-issue run id with no
+  // ownership-marker row, so the API's upsert → getReport →
+  // assertRunOwnership would 404 (surfaced as a bare "HTTP 404:" toast).
+  // The page run id is the one we fetched the report/issues with and that
+  // the securityPenetrationTestRun marker is keyed on.
+  const resolvedRunId = runId ?? null;
 
   const form = useForm<FindingContextForm>({
     resolver: zodResolver(findingContextSchema),