Repository navigation
chore: retire deprecated security-scanning workflow - #6449
Conversation
Remove workflow_dispatch-only security-scanning.yml and point docs, production-readiness checks, and tests at security.yml and security-deep.yml.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideThe PR removes the deprecated workflow and consistently retargets code, tests, security guidance, compliance documentation, and release-readiness fixtures to the consolidated fast security.yml and deep security-deep.yml workflows, without changing either workflow's implementation. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
px Advisor ReviewRisk: unknown No summary available No findings. |
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="apps/web/src/pages/api/v1/production-readiness.ts" line_range="382" />
<code_context>
- existsSync(
- path.join(REPO_ROOT, '.github/workflows/security-scanning.yml'),
- ),
+ existsSync(path.join(REPO_ROOT, '.github/workflows/security.yml')),
existsSync(path.join(REPO_ROOT, 'security-baseline.json')),
existsSync(path.join(SRC_DIR, 'lib/security')),
</code_context>
<issue_to_address>
**Security readiness passes without deep scans**
When `security-deep.yml` is missing while `security.yml`, the baseline, and the security library exist, `checkSecurityStandards` checks only `security.yml`, so its checks still score 100% and the readiness API reports security as passing without CodeQL, Trivy, Checkov, or SBOM scans.
Include `.github/workflows/security-deep.yml` in the security readiness checks.
</issue_to_address>
### Comment 2
<location path="apps/web/src/lib/security/__tests__/security-scanning.test.ts" line_range="68-69" />
<code_context>
- // Check for security scanning components
- expect(workflowContent).toContain('security-scan')
+ expect(workflowContent).toContain('codeql-analysis')
expect(workflowContent).toMatch(/ubuntu-\w+/)
})
</code_context>
<issue_to_address>
**CodeQL can lose its runner check**
When the CodeQL job is changed to a non-Ubuntu runner while another job remains on Ubuntu, when the `codeql-analysis` job uses a non-Ubuntu runner, the test still passes because its Ubuntu regex matches a runner elsewhere in the workflow; the CodeQL configuration check therefore accepts a broken job.
Parse the workflow and assert that the `codeql-analysis` job itself has an Ubuntu `runs-on` value.
</issue_to_address>
### Comment 3
<location path="apps/web/src/lib/security/__tests__/security-scanning.test.ts" line_range="28" />
<code_context>
+ expect(fastWorkflowContent).toContain('name: Security Scanning')
+ expect(deepWorkflowContent).toContain('trivy-action')
+ expect(deepWorkflowContent).toContain('checkov-action')
+ expect(deepWorkflowContent).toContain('sarif')
})
</code_context>
<issue_to_address>
**Scanner SARIF output can disappear unnoticed**
When trivy and Checkov SARIF output is removed while CodeQL SARIF configuration remains, the workflow test still passes because its `sarif` substring check matches CodeQL SARIF configuration elsewhere, even when Trivy and Checkov no longer produce SARIF results.
Assert that the Trivy and Checkov scan steps themselves configure SARIF output.
</issue_to_address>
### Comment 4
<location path=".github/security/container-suppression.md" line_range="78" />
<code_context>
- [Vulnerability Suppression Policy](./vulnerability-suppression.md) —
npm/Python dependency scanning
-- [Security Scanning Workflow](../../workflows/security-scanning.yml) — CI
+- [Security Deep Scan Workflow](../../workflows/security-deep.yml) — CI
pipeline
- [Dependabot Configuration](../../dependabot.yml) — automated dependency
</code_context>
<issue_to_address>
**Deep-scan policy link is broken**
When a reader follows the workflow link, the relative link resolves from `.github/security/` to `workflows/security-deep.yml` at the repository root, not `.github/workflows/security-deep.yml`, so readers following it reach a missing page.
Change the relative path to `../workflows/security-deep.yml`.
</issue_to_address>Sourcery assessment
Approval pending. 3 findings to address first.
Blocking findings: apps/web/src/pages/api/v1/production-readiness.ts:382, apps/web/src/lib/security/__tests__/security-scanning.test.ts:69, apps/web/src/lib/security/__tests__/security-scanning.test.ts:28
Lighthouse CI ResultsRoutes audited: Thresholds: Performance ≥90 | Accessibility ≥90 | Best Practices ≥85 Reports
|
px Advisor ReviewRisk: unknown No summary available No findings. |
| async function checkSecurityStandards(): Promise<ProductionReadinessCheck> { | ||
| try { | ||
| const securityChecks = [ | ||
| existsSync( | ||
| path.join(REPO_ROOT, '.github/workflows/security-scanning.yml'), | ||
| ), | ||
| existsSync(path.join(REPO_ROOT, '.github/workflows/security.yml')), | ||
| existsSync(path.join(REPO_ROOT, '.github/workflows/security-deep.yml')), | ||
| existsSync(path.join(REPO_ROOT, 'security-baseline.json')), | ||
| existsSync(path.join(SRC_DIR, 'lib/security')), | ||
| ] |
There was a problem hiding this comment.
Bug: Inserting a new item into securityChecks caused the indices in details.checks to misalign with their corresponding check labels and omitted the fourth check.
Severity: MEDIUM
Suggested Fix
Update details.checks in checkSecurityStandards to correctly map all four check results, adding a field for 'Deep Security Workflow' at index 1, 'Security Baseline' at index 2, and 'Security Library' at index 3.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: apps/web/src/pages/api/v1/production-readiness.ts#L379-L386
Potential issue: In `checkSecurityStandards`, the `securityChecks` array was expanded to
four items by inserting `.github/workflows/security-deep.yml` at index 1. However, the
`details.checks` mapping was not updated and still references indices 0, 1, and 2 using
the old check labels. As a result, 'Security Baseline' maps to `securityChecks[1]`
(`security-deep.yml`), 'Security Library' maps to `securityChecks[2]`
(`security-baseline.json`), and index 3 (`lib/security`) is completely omitted. When
checks fail, the endpoint returns mismatched status labels.
Did we get this right? 👍 / 👎 to inform future reviews.
Lighthouse CI ResultsRoutes audited: Thresholds: Performance ≥90 | Accessibility ≥90 | Best Practices ≥85 Reports
|
px Advisor ReviewRisk: unknown No summary available No findings. |
Lighthouse CI ResultsRoutes audited: Thresholds: Performance ≥90 | Accessibility ≥90 | Best Practices ≥85 Reports
|
px Advisor ReviewRisk: unknown No summary available No findings. |
Lighthouse CI ResultsRoutes audited: Thresholds: Performance ≥90 | Accessibility ≥90 | Best Practices ≥85 Reports
|
px Advisor ReviewRisk: unknown No summary available No findings. |
Lighthouse CI ResultsRoutes audited: Thresholds: Performance ≥90 | Accessibility ≥90 | Best Practices ≥85 Reports
|
Summary
Removes the deprecated
workflow_dispatch-only security workflow and retargets references to the consolidated live workflows.Deletion
.github/workflows/security-scanning.yml— marked DEPRECATED in-file; all jobs already live insecurity.yml(fast) andsecurity-deep.yml(deep). No other workflowuses:this file (verified via repo search).Retargeted references
apps/web/src/pages/api/v1/production-readiness.ts.github/workflows/security.ymlapps/web/src/lib/security/__tests__/security-scanning.test.tssecurity.yml+security-deep.yml(fast vs deep responsibilities).github/security/vulnerability-suppression.mdpnpm audit/pip-audit→security.yml; Checkov / Syft →security-deep.yml(job names updated).github/security/container-suppression.mdsecurity-deep.ymlapps/web/src/content-store/docs/security/compliance.mdsecurity.yml+security-deep.ymlapps/web/src/content-store/docs/security/hipaa-compliance.mdsecurity.yml+security-deep.ymlci-cd/release-readiness.jsonsecurity-scanning→securityscripts/ci/release-readiness-aggregator.pySkipped / unchanged
security.ymlandsecurity-deep.yml— no logic changes (per scope).Verification
pnpm lint— pass (Node 24.20.0)pnpm vitest run -c config/vitest.config.ts apps/web/src/lib/security/__tests__/security-scanning.test.ts— 5/5 passrg security-scanning— no matches remainingSummary by Sourcery
Remove the deprecated security-scanning workflow and migrate all remaining references to the active security workflows.
Enhancements:
Documentation:
Tests:
Chores: