Skip to content

fix(evidence-forms): store uploads under the form's own file field - #3575

Open
0xtechdean wants to merge 3 commits into
trycompai:mainfrom
0xtechdean:fix/evidence-upload-file-field-key-clean
Open

0xtechdean wants to merge 3 commits into
trycompai:mainfrom
0xtechdean:fix/evidence-upload-file-field-key-clean

Conversation

@0xtechdean

@0xtechdean 0xtechdean commented Sep 22, 2026 •

Copy link
Copy Markdown

What does this PR do?

uploadSubmission wrote every uploaded file to a hardcoded evidenceFile key, regardless of form type. The submission detail view renders each field by its declared key (submission.data[field.key]), so for any form whose file field is named something else, the real field stayed empty — the submission rendered — against every field, including the one its own submission schema marks required.

The file wasn't lost: CompanySubmissionDetailPageClient has a fallback that appends a generic Uploaded Evidence row when data.evidenceFile is present and the form declares no field by that name. That row is why this looks superficially fine while the form's actual fields are blank.

Only two forms actually declare a field called evidenceFile — whistleblower-report and tabletop-exercise. Every other form with a file field was storing under a key nothing reads:

Form Declared file field Written as (before)
penetration-test pentestReport (required) evidenceFile
network-diagram diagramFile evidenceFile
rbac-matrix matrixFile evidenceFile

So an uploaded penetration test report displayed with no Test date, Vendor / Testing Firm, Summary of Findings or Pentest Report against it — just a bare file link. For network-diagram the stored row doesn't satisfy its own schema, which requires either diagramUrl or diagramFile.

The fix

Resolve the target key from the form definition rather than hardcoding it, preferring a required file field when a form declares more than one. Forms that declare no file field keep the generic key, so their behaviour is unchanged.

The generic fallback row is deliberately left in place — it still serves forms with no file field, and existing rows already written under evidenceFile.

Existing data

Included as a backfill migration (20260922120000_backfill_evidence_upload_file_field), following the existing backfill_* convention. For the three affected form types it copies the file onto the declared key, then drops the generic key so the file renders once under its proper field rather than twice — the fallback row would otherwise still fire, since those forms declare no field named evidenceFile.

It is deliberately conservative: it only writes where the declared key is absent, and only removes evidenceFile after confirming the copy matches, so it is idempotent and safe to re-run. whistleblower-report and tabletop-exercise (which genuinely declare evidenceFile) and the form types with no file field are untouched.

Verified against a real database inside a rolled-back transaction: a simulated pre-fix row converts correctly, the file object is preserved intact, and a second run is a no-op (UPDATE 0).

Visual Demo

N/A — the change is to which key the submission payload is written under. The behavioural difference is covered by tests: reverting the resolver to the old hardcoded key fails 5 of them.

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code.
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. N/A — no documented behaviour changes; the form definitions are already the source of truth.
  • I have added tests: resolveUploadFileFieldKey unit tests (including an invariant over every registered form type) plus uploadSubmission service tests asserting the file lands on the declared field and that the resulting submission passes its own schema. npx jest src/evidence-forms → 36 passed, 4 suites.

`uploadSubmission` wrote every uploaded file to a hardcoded `evidenceFile`
key. The submission detail view renders each field by its declared key, so
for any form that names its file field something else the real field stayed
empty and the submission displayed "—" against every field — including the
one its own submission schema marks required. The file itself was reachable
only through the generic "Uploaded Evidence" fallback row.

Only `whistleblower-report` and `tabletop-exercise` actually declare a field
called `evidenceFile`. `penetration-test` (`pentestReport`), `network-diagram`
(`diagramFile`) and `rbac-matrix` (`matrixFile`) were each stored under a key
nothing reads, so an uploaded penetration test report rendered with no test
date, vendor or summary against it.

Resolve the target key from the form definition instead, preferring a
required file field, and keep the generic key for forms that declare no file
field at all so their behaviour is unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@CLAassistant

CLAassistant commented Sep 22, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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

No issues found across 4 files

Confidence score: 5/5

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

Re-trigger cubic

Companion data migration for the upload-path fix. For the three form types
whose file field is not named `evidenceFile` — penetration_test, network_diagram
and rbac_matrix — copy the stored file onto the declared key, then drop the
generic key so the file renders once under its own field instead of twice
through the "Uploaded Evidence" fallback row.

Only writes where the declared key is absent, and only removes `evidenceFile`
after confirming the copy matches, so the migration is idempotent. Form types
that genuinely declare `evidenceFile` (whistleblower_report, tabletop_exercise)
and those with no file field are left untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@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 1 file (changes from recent commits).

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

Fix all with cubic | Re-trigger cubic

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/db/prisma/migrations/20260922120000_backfill_evidence_upload_file_field/migration.sql">

<violation number="1" location="packages/db/prisma/migrations/20260922120000_backfill_evidence_upload_file_field/migration.sql:23">
P0: This migration fails before backfilling any rows because PostgreSQL casts each string literal to `EvidenceFormType`, and the underscore names are not valid enum labels. Use `penetration-test`, `network-diagram`, and `rbac-matrix` in all six `WHERE` clauses.</violation>
</file>

…tion

The migration filtered on `penetration_test` / `network_diagram` /
`rbac_matrix`, but `EvidenceFormType` is declared with `@map`, so the labels
actually stored in Postgres are hyphenated. Casting the underscore form raises
`invalid input value for enum "EvidenceFormType"`, which would have aborted the
migration before it touched a single row.

Use the stored labels. Verified by running the migration file itself against
rows seeded into the pre-fix shape: both convert correctly and a second run is
a no-op.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@0xtechdean

Copy link
Copy Markdown
Author

Good catch on the enum labels — that was a real defect and it would have aborted the whole migration before touching a row, not silently skipped.

Root cause: EvidenceFormType is declared with @map, so the Prisma client names are underscored but the labels stored in Postgres are hyphenated. Casting 'penetration_test' raises invalid input value for enum "EvidenceFormType".

Fixed in bc952b9 — the filters now use the stored labels (penetration-test, network-diagram, rbac-matrix).

My original verification missed it because I tested the jsonb transform without the formType predicate. Re-verified properly by running the migration file itself against rows seeded back into the pre-fix shape: both convert to the declared key, the file object is preserved, and a second run is a no-op (all UPDATE 0).

This branch has not been deployed

No deployments
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.

2 participants