Skip to content

HYPERFLEET-1188 - docs: add Snyk SAST sign-off for v1.0.0#161

Open
kuudori wants to merge 1 commit into
openshift-hyperfleet:mainfrom
kuudori:HYPERFLEET-1188
Open

HYPERFLEET-1188 - docs: add Snyk SAST sign-off for v1.0.0#161
kuudori wants to merge 1 commit into
openshift-hyperfleet:mainfrom
kuudori:HYPERFLEET-1188

Conversation

@kuudori

@kuudori kuudori commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Documented review and triage of all 22 Snyk Code (SAST) findings across hyperfleet-api, hyperfleet-sentinel, and hyperfleet-adapter. No unaddressed security-scan defects — 20 test noise, 1 false positive, 1 true positive accepted by design (guarded TLS opt-in). Follow-up tickets filed: HYPERFLEET-1233, HYPERFLEET-1234.

Summary

Test Plan

  • Unit tests added/updated
  • make test-all passes
  • make lint passes
  • Helm chart changes validated with make test-helm (if applicable)
  • Deployed to a development cluster and verified (if Helm/config changes)
  • E2E tests passed (if cross-component or major changes)

Documented review and triage of all 22 Snyk Code (SAST) findings
across hyperfleet-api, hyperfleet-sentinel, and hyperfleet-adapter.
No unaddressed security-scan defects — 20 test noise, 1 false
positive, 1 true positive accepted by design (guarded TLS opt-in).
Follow-up tickets filed: HYPERFLEET-1233, HYPERFLEET-1234.
@openshift-ci openshift-ci Bot requested review from Mischulee and pnguyen44 June 16, 2026 16:30
@openshift-ci

openshift-ci Bot commented Jun 16, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign vkareh for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

A new Markdown document is added at hyperfleet/docs/release/v1.0.0/sast-signoff.md. It records the Snyk SAST scan performed against HyperFleet v1.0.0, covering scan scope and known limitations (advisory mode, severity thresholding, false-positive filtering), scan metadata (date, tool version, OCI retrieval method, related ticket), scanned image digests per component and commit, per-component findings tables with verdicts (Noise, False positive, Accepted) and rationale, follow-up hardening/hygiene ticket mappings, and a reviewer attestation statement that explicitly accepts one guarded true positive finding.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


Findings to verify against the document:

  • Accepted true positive: The sign-off explicitly accepts one guarded true positive. Confirm the rationale is sufficient and a compensating control or follow-up ticket is recorded. Absent a compensating control, this is an open CWE that persists into the released image.
  • Verdict rationale completeness: For each finding marked "Noise" or "False positive," confirm the rationale cites a specific code path, configuration, or unreachable condition — not a generic dismissal. Undocumented false-positive dismissals weaken the audit trail for future CWE/CVE triage.
  • Image digests: Verify the recorded OCI digests match the actually shipped images. A mismatch means the scan results do not correspond to the released artifacts — the sign-off provides no assurance.
  • Scope limitations clause: Advisory mode and severity thresholding mean findings below the configured threshold are not triaged. Confirm the threshold is documented and that no suppressed-by-threshold findings carry CWE classifications relevant to the Go/K8s attack surface (e.g., CWE-89, CWE-78, CWE-918, CWE-200, CWE-295).
  • hyperfleet-sentinel zero findings: A clean result warrants scrutiny. Confirm coverage was not vacuous (e.g., empty scan scope, excluded paths, or no Go source in the analyzed image layer).
🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Title check ✅ Passed The title directly references the ticket (HYPERFLEET-1188) and accurately describes the addition of Snyk SAST sign-off documentation for v1.0.0, matching the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Sec-02: Secrets In Log Output ✅ Passed PR contains only documentation (125 .md files, configs, images). Zero Go source/test files present; no log statements exist to audit for secrets.
No Hardcoded Secrets ✅ Passed Documentation file contains no hardcoded secrets. References to test fixtures (testpass, testuser) and commit/digest hashes are documented findings triage, not embedded credentials. All sensitive p...
No Weak Cryptography ✅ Passed Documentation-only PR. SAST scan found no banned crypto primitives (MD5, DES, RC4, SHA1 for security), ECB mode, custom crypto, or non-constant-time comparisons. Findings were credential/certificat...
No Injection Vectors ✅ Passed PR contains only documentation (sast-signoff.md). Snyk SAST review found zero injection findings: no CWE-89 (SQL), CWE-78 (command), CWE-79 (XSS), or CWE-502 (deserialization) defects reported acro...
No Privileged Containers ✅ Passed PR adds only documentation (sast-signoff.md); no deployment manifests, Helm templates, or Dockerfiles modified. Zero privileged container configurations found.
No Pii Or Sensitive Data In Logs ✅ Passed Documentation-only change: no logging statements present. Check for PII/sensitive data in logs is not applicable to markdown documents without executable code.
Description check ✅ Passed The PR description directly addresses the changeset: it documents the triage of 22 Snyk Code SAST findings across three hyperfleet components with specific verdicts (20 noise, 1 false positive, 1 accepted true positive) and references the corresponding Jira ticket HYPERFLEET-1188.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

Comment @coderabbitai help to get the list of available commands and usage tips.


## Scope

This document records the review and triage of all Snyk Code (SAST) findings for the HyperFleet v1.0.0 release candidate. It covers source-code static analysis only — penetration testing, threat modeling, and container image vulnerability scanning (Clair/Pyxis) are handled separately under the existing Section 4.5 release gate.

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.

nit (medium confidence): This sentence attributes pen testing, threat modeling, and container scanning to "Section 4.5 release gate," but Section 4.5 of the release process only covers container vulnerability scanning, supply chain security, SBOM, signing, and EC policy — pen testing and threat modeling aren't referenced in any documented gate.

Consider splitting the clause and linking the reference:

Suggested change
This document records the review and triage of all Snyk Code (SAST) findings for the HyperFleet v1.0.0 release candidate. It covers source-code static analysis only — penetration testing, threat modeling, and container image vulnerability scanning (Clair/Pyxis) are handled separately under the existing Section 4.5 release gate.
This document records the review and triage of all Snyk Code (SAST) findings for the HyperFleet v1.0.0 release candidate. It covers source-code static analysis only — container image vulnerability scanning is handled under the existing [Section 4.5 release gate](../hyperfleet-release-process.md#45-security--compliance); penetration testing and threat modeling are out of scope.


| Ticket | Type | Component | Scope |
|--------|------|-----------|-------|
| [HYPERFLEET-1233](https://redhat.atlassian.net/browse/HYPERFLEET-1233) | Hardening | hyperfleet-adapter | Finding #17: add startup warning log for insecure TLS mode, enrich `//nolint:gosec` comment with rationale. Findings #18–22: test credential cleanup. |

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.

nit (medium confidence): "Findings #18–22: test credential cleanup" groups credential findings (#18–19, HardcodedPassword, CWE-798) with TLS configuration findings (#20–22, TooPermissiveTrustManager, CWE-295). The latter are about permissive TLS in envtest fixtures, not credentials.

Suggested change
| [HYPERFLEET-1233](https://redhat.atlassian.net/browse/HYPERFLEET-1233) | Hardening | hyperfleet-adapter | Finding #17: add startup warning log for insecure TLS mode, enrich `//nolint:gosec` comment with rationale. Findings #1822: test credential cleanup. |
| [HYPERFLEET-1233](https://redhat.atlassian.net/browse/HYPERFLEET-1233) | Hardening | hyperfleet-adapter | Finding #17: add startup warning log for insecure TLS mode, enrich `//nolint:gosec` comment with rationale. Findings #1819: test credential cleanup. Findings #20–22: add `//nolint` annotations for expected permissive TLS in envtest fixtures. |


### hyperfleet-sentinel

No findings.

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.

Tip

nit — non-blocking suggestion

Category: Improvement

Zero findings is the one result where a brief coverage note adds audit value — it confirms the scan wasn't vacuous. Consider adding a parenthetical like:

Suggested change
No findings.
No findings. SARIF report confirmed well-formed with scan exit code 0.

Without this, a future auditor can't distinguish "clean code" from "empty scan scope" — unlike api and adapter, where non-zero findings implicitly prove the scan ran.


### Limitations

Results are from CI-produced SARIF (Konflux pipeline), which applies `--severity-threshold=high` and Red Hat Known False Positive (KFP) filtering. Lower-severity findings that would appear in a local `snyk code test` run may be suppressed. The assurance in this sign-off is bounded by what the CI scan was configured to report.

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.

Tip

nit — non-blocking suggestion

Category: Improvement

All 22 findings in this doc are at note or warning SARIF severity — both below high. This creates a confusing reading when the same paragraph says --severity-threshold=high is applied: a reader can't tell whether findings below note were silently dropped, or whether the threshold only affects the pipeline gate (pass/fail) and the SARIF from oras pull contains everything.

Consider clarifying what the threshold actually governs:

Suggested change
Results are from CI-produced SARIF (Konflux pipeline), which applies `--severity-threshold=high` and Red Hat Known False Positive (KFP) filtering. Lower-severity findings that would appear in a local `snyk code test` run may be suppressed. The assurance in this sign-off is bounded by what the CI scan was configured to report.
Results are from CI-produced SARIF (Konflux pipeline). The pipeline applies `--severity-threshold=high` to govern the build gate (pass/fail exit code); the SARIF retrieved via OCI referrers contains findings at all severity levels. Red Hat Known False Positive (KFP) filtering is also applied, which may suppress findings that would appear in a local `snyk code test` run. The assurance in this sign-off is bounded by what the CI scan was configured to report.

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

A couple of nits

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.

3 participants