Skip to content

feat(myopencre): improve CSV import validation (export-compatible)#682

Merged
northdpole merged 11 commits intoOWASP:mainfrom
PRAteek-singHWY:myopencre-csv-import-validation
Feb 26, 2026
Merged

feat(myopencre): improve CSV import validation (export-compatible)#682
northdpole merged 11 commits intoOWASP:mainfrom
PRAteek-singHWY:myopencre-csv-import-validation

Conversation

@PRAteek-singHWY
Copy link
Contributor

@PRAteek-singHWY PRAteek-singHWY commented Dec 28, 2025

⚠️ This PR depends on:


Note for reviewers

The intended change in this PR is limited to
application/web/web_main.py (CSV import validation logic).

Any additional file diffs shown by GitHub are inherited from branch history / stacking and are not part of the validation change itself.

Please let me know if you’d prefer this PR to be rebased or split into a narrower diff.


Summary

Closes #584 (partial)

This PR improves server-side validation for the MyOpenCRE CSV import endpoint, aligning it closely with the CSV format produced by the CRE catalogue export flow.

The intent is not to introduce strict or surprising validation rules, but to ensure that:

  • every CSV produced by the exporter can be safely imported
  • malformed or ambiguous input is rejected early with clear, structured errors
  • exporter artifacts (padding rows, empty rows) do not cause failures

What this PR does

File-level validation

  • Rejects missing, empty, non-CSV, or non–UTF-8 uploads
  • Returns consistent, structured error responses

Schema / header validation

  • Requires at least one CRE* column
  • Requires standard|name and standard|id
  • Rejects rows with more columns than the header (misaligned CSVs)

Row-level validation (export-compatible)

  • Completely empty rows are ignored (exported templates include padding rows)
  • Rows without any CRE values are ignored
  • CRE entries are validated only when present (<CRE-ID>|<Name>)
  • Malformed CRE entries return row-specific validation errors

No-op import guard

  • If a file contains no importable rows after filtering, the request returns success with no changes
  • This avoids confusing partial imports or unnecessary failures

What is intentionally ignored (by design)

  • Empty rows
  • Padding rows from exported templates
  • Rows without CRE mappings
  • Extra unused columns (as long as the CSV structure itself is valid)

This mirrors how production importers typically behave and allows exported files to be round-tripped without manual cleanup.


What is out of scope for this PR

  • Frontend error presentation / formatting
  • UX changes around errors

These will be handled in follow-up PRs to keep this change focused and reviewable.


Why this approach

The importer now accepts everything the exporter produces, ignores exporter padding rows, and enforces validation only where semantic meaning exists.

This keeps the import process resilient while still preventing invalid data from entering the system.


Dependency note

This PR is stacked on top of:

feat(myopencre): add CSV upload UI and wire to existing import endpoint (#664)

It should be reviewed and merged after that PR.


Feedback welcome

If any validation behavior is too permissive or too strict, or if there are exporter edge cases I may have missed, I’d really appreciate guidance and am happy to adjust.

@PRAteek-singHWY
Copy link
Contributor Author

Hi! @northdpole Sir I spent a few hours validating this against the CSV produced by the CRE catalogue exporter and tried to keep the importer behavior as close to that format as possible.

If any of the validation rules here feel too permissive, too strict, or misaligned with intended usage, I’d really appreciate feedback and guidance.

@PRAteek-singHWY PRAteek-singHWY force-pushed the myopencre-csv-import-validation branch from 53b85b0 to 197653b Compare January 3, 2026 23:25
@PRAteek-singHWY
Copy link
Contributor Author

Thanks a lot @northdpole Sir for the thoughtful feedback — much appreciated 👍

I’ve addressed the concern about validation placement across this PR stack .

What changed based on your feedback
• CSV validation logic has been moved out of the controller layer and into the spreadsheet parsing layer.
• The parser entry point (parse_export_format) now acts as the single place where incoming CSVs are validated and parsed.
• Structural and row-level validation rules live in a dedicated helper (validate_import_csv_rows), which is invoked internally by the parser before any import logic proceeds.

This keeps the controller focused strictly on request/response handling, while centralizing CSV validation in a place that’s easier to reason about, test, and evolve (with a clear path toward a standalone validation utility if needed).

PR overview (for context)
#682 – Backend CSV import validation aligned with export format
#683 – Frontend handling and display of validation errors
#684 – Clear UI messaging for no-op / empty imports
#685 – CSV import preview and confirmation flow for safer imports
#686 – Inline help and guidance for CSV preparation

Each PR builds incrementally on the previous one, but validation itself now consistently happens at the parser level, not in the controllers.

I’m definitely looking forward to improving the validation further — especially around clearer error semantics and extensibility as more CSV use-cases come in.

Thanks again for the guidance, it helped clean up the design significantly.
Happy to adjust naming or structure further if you’d prefer a different direction.

@PRAteek-singHWY PRAteek-singHWY force-pushed the myopencre-csv-import-validation branch 3 times, most recently from fec5903 to 67bff0d Compare February 14, 2026 21:48
@PRAteek-singHWY PRAteek-singHWY force-pushed the myopencre-csv-import-validation branch from 67bff0d to 9ec590d Compare February 26, 2026 20:01
@PRAteek-singHWY PRAteek-singHWY force-pushed the myopencre-csv-import-validation branch from 9ec590d to 1e71021 Compare February 26, 2026 21:53
@northdpole northdpole merged commit 605bb6d into OWASP:main Feb 26, 2026
2 checks passed
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.

MyOpenCRE: create frontend

2 participants