Skip to content

Add Trivy security-audit workflow and report sorter; validate cgroup paths against embedded NUL#26

Draft
andypost wants to merge 1 commit into
roadmapfrom
codex/create-security-audit-with-sorted-results
Draft

Add Trivy security-audit workflow and report sorter; validate cgroup paths against embedded NUL#26
andypost wants to merge 1 commit into
roadmapfrom
codex/create-security-audit-with-sorted-results

Conversation

@andypost
Copy link
Copy Markdown
Owner

Motivation

  • Add an automated Trivy filesystem vulnerability audit that produces a human-readable, severity-sorted Markdown report and can fail the run on configurable severity.
  • Harden cgroup path validation to reject empty values and embedded NUL bytes that could lead to truncated C-string usage.
  • Capture findings and follow-up items for the new audit flow in documentation and exercise the new validation with a unit test.

Description

  • Add .github/scripts/sort-trivy-results.py which parses Trivy JSON output, sorts vulnerabilities by severity and CVSS v3 score, renders a Markdown report, and can exit non-zero when a configured severity threshold is met.
  • Add .github/workflows/security-audit.yml GitHub Action to run Trivy (JSON and SARIF), upload SARIF, generate the sorted Markdown report, append it to the step summary, and upload the security-audit/ artifact.
  • Add docs/security-audit-findings.md documenting candidate findings from a manual review of the new audit/workflow and adjacent code paths.
  • Harden C validation in src/nxt_conf_validation.c for cgroup paths by rejecting empty strings and embedded NUL bytes, replacing a sprintf with snprintf to avoid unsafe formatting, and preserving the existing ".." path check.
  • Extend test/test_python_isolation.py to include a test case that asserts a cgroup path containing an embedded NUL ('scope\0python') is treated as invalid.

Testing

  • Updated isolation unit test test/test_python_isolation.py::test_python_isolation_cgroup_invalid was run and the new case for embedded NULs passed.
  • The repository unit test suite was executed after the change via the project test runner and completed successfully.
  • The new GitHub Actions workflow was added and its configuration validated via static review; the workflow will run in CI on pushes, PRs, and a weekly schedule.

Codex Task

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds a Python script to generate severity-sorted Markdown reports from Trivy JSON output and includes a new document tracking potential security vulnerabilities. Additionally, it hardens cgroup path validation in src/nxt_conf_validation.c by rejecting embedded NUL bytes and using snprintf, with a corresponding test case added. Review feedback suggests enhancing the Markdown escaping logic to handle backslashes and removing redundant severity normalization in the sorting process.


def markdown_escape(value: Any) -> str:
text = "" if value is None else str(value)
return text.replace("|", "\\|").replace("\n", " ").replace("\r", " ")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The current escaping logic does not handle backslashes. In Markdown tables, a backslash preceding a pipe (\|) acts as an escape character for the pipe. If a field (such as a package title or version) ends with a backslash, it could inadvertently escape the table delimiter and break the report formatting. It is safer to escape backslashes first.

Suggested change
return text.replace("|", "\\|").replace("\n", " ").replace("\r", " ")
return text.replace("\\", "\\\\").replace("|", "\\|").replace("\n", " ").replace("\r", " ")



def vulnerability_sort_key(vulnerability: dict[str, Any]) -> tuple[Any, ...]:
severity = normalized_severity(vulnerability.get("Severity"))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The Severity field is already normalized in the collect_vulnerabilities function (line 101). Re-calling normalized_severity here is redundant and adds unnecessary overhead during the sorting process, especially for large reports.

Suggested change
severity = normalized_severity(vulnerability.get("Severity"))
severity = vulnerability.get("Severity", "UNKNOWN")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant