Skip to content

fix(submission): Auto-transform ProtectedDonor workbooks during validate/submit - #48

Open
aschroed wants to merge 10 commits into
masterfrom
fm/protected-donor-submitr
Open

fix(submission): Auto-transform ProtectedDonor workbooks during validate/submit#48
aschroed wants to merge 10 commits into
masterfrom
fm/protected-donor-submitr

Conversation

@aschroed

@aschroed aschroed commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Intent

Add selective ProtectedDonor workbook analysis and transformation across dcicutils and submitr. Protected-item sheets are Demographic, DeathCircumstances, FamilyHistory, MedicalHistory, and TissueCollection. Mixed Donor and ProtectedDonor references are supported; missing workbook Donors fail clearly; existing ProtectedDonors may come from the workbook or portal; unreferenced Donors remain unchanged; and validate/submit perform the transformation without an interactive transform prompt.

What changed

  • Added submitr integration for ProtectedDonor analysis and selective workbook transformation.
  • Added hidden internal compatibility switches for transformed-workbook handling.
  • Added automatic transformed-workbook reporting for validation and submission.
  • Added focused tests for transformation, mixed references, missing references, CLI wiring, and already-transformed handling.

Remediation updates

  • Use the mapped StructuredDataSet.data serialized to temporary JSON as the canonical remote validation/submission payload, so CustomExcel mappings such as qc_values#... are preserved. The transformed .xlsx remains available as the user-visible artifact.
  • Protect existing transformed-workbook targets and replace them atomically only after successful transformation and validation; failed validation no longer destroys the previous output.
  • Analyze the same cached-value workbook representation used by normal CustomExcel ingestion. Formula-backed donor references are accepted when valid cached values are present; formulas without usable cached values fail clearly rather than being evaluated by submitr.
  • Report malformed workbook-load failures with their context instead of silently skipping the ProtectedDonor decision.
  • Keep the dcicutils dependency coordinated with the beta ProtectedDonor implementation in Fm/protected donor dcicutils 4dn-dcic/utils#339.

Testing

Focused submitr tests cover the ProtectedDonor transform, mapped upload payload, cached formula behavior, output preservation/replacement, and malformed-workbook handling. The dcicutils changes and tests are in 4dn-dcic/utils#339.

This PR is intentionally not merged automatically; the dcicutils beta implementation/release must remain coordinated with this branch.

@dbmi-svc-checkmarx

dbmi-svc-checkmarx commented Aug 20, 2026

Copy link
Copy Markdown

Logo
Checkmarx One – Scan Summary & Details1d097065-bbd0-4e39-a8c6-585a9065d091


New Issues (3) Checkmarx found the following issues in this Pull Request
# Severity Issue Source File / Package Checkmarx Insight
1 HIGH CVE-2026-69249 Python-cryptography-43.0.3
detailsRecommended version: 49.0.0
Description: python-cryptography is a package designed to expose cryptographic primitives and recipes to Python developers. Prior to 49.0.0, when resolving inva...
Attack Vector: NETWORK
Attack Complexity: LOW
Vulnerable Package
2 MEDIUM CVE-2026-69248 Python-cryptography-43.0.3
detailsRecommended version: 49.0.0
Description: cryptography is a package designed to expose cryptographic primitives and recipes to Python developers. Prior to 49.0.0, if an intermediate constra...
Attack Vector: NETWORK
Attack Complexity: LOW
Vulnerable Package
3 MEDIUM Unchecked_Input_for_Loop_Condition submitr/submission.py: 3844
detailsMethod at line 3844 of /submitr/submission.py obtains user input from load - the range of this value is not validated, and is eventually used in...
Attack Vector

Use @Checkmarx to interact with Checkmarx PR Assistant.
Examples:
@Checkmarx how are you able to help me?
@Checkmarx rescan this PR

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