Skip to content

fix(security): complete allowlist input validation audit - #233

Open
TFT444 wants to merge 2 commits into
devfrom
fix/201-input-validation-audit
Open

fix(security): complete allowlist input validation audit#233
TFT444 wants to merge 2 commits into
devfrom
fix/201-input-validation-audit

Conversation

@TFT444

@TFT444 TFT444 commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • add centralized allowlist, type, format, and size validation for API inputs
  • validate scan/findings paths, query parameters, AI payloads, request IDs, auth headers, and Sentinel ingestion data
  • document every reviewed boundary and update the OpenSSF Silver evidence register
  • add security regression coverage for malformed, oversized, duplicated, and injection-style inputs

Validation

  • ruff check .
  • ruff format --check .
  • 511 passed, 3 skipped with 84.42% API/scanner coverage
  • frontend lint, i18n, accessibility, and production build
  • website URL security tests
  • Bandit: no medium/high findings
  • pip-audit and both npm audits: 0 vulnerabilities

Closes #201

Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Comment thread api/routes/ai.py Fixed
Comment thread api/routes/ai.py Fixed
Comment thread api/routes/ai.py Fixed
Comment thread api/routes/ai.py Fixed
Comment thread api/routes/ai.py Fixed
Comment thread api/routes/findings.py Fixed
Comment thread api/routes/findings.py Fixed
Comment thread api/routes/scans.py Fixed
Comment thread api/routes/scans.py Fixed
Comment thread api/routes/scans.py Fixed
@TFT444
TFT444 requested review from m-khan-97 and removed request for parthrohit22 August 6, 2026 11:35
Signed-off-by: Tanvir Farhad <tamimtarafder12@gmail.com>
@TFT444 TFT444 self-assigned this Aug 7, 2026
@Vishnu2707

Copy link
Copy Markdown
Member

@parthrohit22 or @SHAURYAKSHARMA24, can u review this PR?

@Vishnu2707
Vishnu2707 requested review from SHAURYAKSHARMA24 and parthrohit22 and removed request for Vishnu2707, m-khan-97 and ritiksah141 August 8, 2026 13:45

@parthrohit22 parthrohit22 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

overall nice work on this @TFT444 the validator module is clean and I like that you're returning a
generic error instead of leaking the exception text. Pulled it down and ran it: your tests
pass, full suite and ruff are fine on my end.

One thing blocking though. ?category= is now case-sensitive, but the query underneath
still does LOWER(category) = LOWER(%s), so anything lowercase that worked before now
400s — ?category=network, ?category=storage, ?category=keyvault all break. You
handled this on severity with case="upper", category just missed it. Can't use .title()
since it'd mangle "Key Vault", so probably a lowercase-keyed lookup back to the canonical
value. Worth a test pinning it too.

Couple of smaller things while you're in there:

  • /api/compliance/<framework> still echoes the raw input back in its error and doesn't
    go through the new validators — feels like it should, given it's in scope.
  • validate_config() in the Sentinel CLI isn't wrapped, so a missing env var dumps a
    traceback now. Also one bad record kills the whole batch, which is fine as a decision,
    just worth documenting.

Rest looks good. Fix the category thing and I'm happy with it.

Reference : here's what I checked on my end: pulled the branch and ran your two new
test files (28 passed) plus the full suite — 506 passed, 3 skipped. The 5 failures I hit
are already there on dev from missing optional deps locally, nothing to do with your
changes. ruff check and ruff format --check both clean. Also poked at the routes
manually with some junk input — SQL-ish strings, script tags, non-canonical UUIDs — and
they all 400 before touching the DB, with nothing reflected back. Checked the category
allowlist against every CATEGORY constant in scanner/rules/ too, all 9 are covered.
That's how the case-sensitivity thing turned up — compared behaviour side by side
against dev.

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.

security: complete allowlist-based input validation audit

4 participants