Skip to content

Commit 0f950e3

Browse files
christopherpaquinchristopherpaquin
committed
Add Security CI workflow and comprehensive documentation
- Enhanced security-ci.yml with improved formatting and artifact handling - Added OSV-Scanner JSON output and artifact upload - Added if: always() to SARIF upload for reliability - Created docs/security-ci-review.md with integration guidance - Updated README.md to document Security CI workflow - Documented parallel execution pattern (recommended approach) - Updated security features table with CI scanning capabilities - Fixed markdown linting issues (line length, indentation, code blocks) - All pre-commit checks passing
1 parent 37dc9eb commit 0f950e3

3 files changed

Lines changed: 323 additions & 4 deletions

File tree

‎.github/workflows/security-ci.yml‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
name: Security CI
2+
3+
on:
4+
pull_request:
5+
push:
6+
branches: ["main"]
7+
workflow_dispatch:
8+
9+
permissions:
10+
contents: read
11+
security-events: write # Required for SARIF uploads to GitHub Security
12+
13+
jobs:
14+
gitleaks:
15+
name: Secret scan (Gitleaks)
16+
runs-on: ubuntu-latest
17+
steps:
18+
- name: Checkout
19+
uses: actions/checkout@v4
20+
with:
21+
fetch-depth: 0 # Scan full git history for secrets
22+
23+
- name: Run Gitleaks
24+
uses: gitleaks/gitleaks-action@v2
25+
# By default, fails the job when leaks are found
26+
# This complements detect-secrets.sh in pre-commit
27+
28+
semgrep:
29+
name: SAST (Semgrep CE)
30+
runs-on: ubuntu-latest
31+
steps:
32+
- name: Checkout
33+
uses: actions/checkout@v4
34+
35+
- name: Run Semgrep
36+
uses: returntocorp/semgrep-action@v1
37+
with:
38+
config: >-
39+
p/security-audit
40+
p/owasp-top-ten
41+
generateSarif: "1"
42+
43+
- name: Upload SARIF to GitHub Security
44+
uses: github/codeql-action/upload-sarif@v3
45+
if: always() # Upload even if Semgrep finds issues
46+
with:
47+
sarif_file: semgrep.sarif
48+
49+
osv-scanner:
50+
name: Dependency vulns (OSV-Scanner)
51+
runs-on: ubuntu-latest
52+
steps:
53+
- name: Checkout
54+
uses: actions/checkout@v4
55+
56+
- name: OSV-Scanner
57+
uses: google/osv-scanner-action@v2
58+
with:
59+
scan-args: |-
60+
-r .
61+
--format json
62+
--output osv-results.json
63+
64+
- name: Upload OSV results
65+
if: always()
66+
uses: actions/upload-artifact@v4
67+
with:
68+
name: osv-scan-results
69+
path: osv-results.json
70+
retention-days: 7

‎README.md‎

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -573,8 +573,14 @@ If secrets are detected, the commit will be blocked. Use example placeholders li
573573

574574
### CI/CD Configuration
575575

576-
CI workflows are defined in `.github/workflows/ci.yaml`. The workflow consists
577-
of two jobs that run sequentially:
576+
This framework includes **two complementary CI workflows** that run in parallel:
577+
578+
1. **`ci.yaml`**: Code quality and testing (pre-commit checks, unit tests)
579+
2. **`security-ci.yml`**: Security scanning (secret detection, SAST, dependency scanning)
580+
581+
#### Main CI Workflow (`ci.yaml`)
582+
583+
The main CI workflow consists of two jobs that run sequentially:
578584

579585
**Job 1: Pre-commit Checks** (runs first):
580586

@@ -601,6 +607,42 @@ of two jobs that run sequentially:
601607
- ⚡ **Efficient**: Tests only run if pre-commit passes
602608
- 🔄 **Consistent**: Same checks run locally and in CI
603609

610+
#### Security CI Workflow (`security-ci.yml`)
611+
612+
The security CI workflow runs **in parallel** with the main CI workflow and
613+
provides comprehensive security scanning:
614+
615+
**Job 1: Gitleaks Secret Scanning**:
616+
617+
- ✅ Scans full git history for secrets and credentials
618+
- ✅ Complements pre-commit `detect-secrets.sh` (defense in depth)
619+
- ✅ Catches secrets that may have slipped through pre-commit
620+
- ✅ Fails the workflow if secrets are detected
621+
622+
**Job 2: Semgrep SAST**:
623+
624+
- ✅ Static Application Security Testing (SAST)
625+
- ✅ Security audit rules (`p/security-audit`)
626+
- ✅ OWASP Top 10 vulnerability detection (`p/owasp-top-ten`)
627+
- ✅ Uploads results to GitHub Security tab (SARIF format)
628+
629+
**Job 3: OSV-Scanner Dependency Scanning**:
630+
631+
- ✅ Scans dependencies for known vulnerabilities
632+
- ✅ Uses Open Source Vulnerabilities (OSV) database
633+
- ✅ Generates JSON report with findings
634+
- ✅ Uploads results as workflow artifact
635+
636+
**Key Features**:
637+
638+
- 🔒 **Defense in Depth**: Multiple layers of security scanning
639+
- 📊 **GitHub Integration**: SARIF results appear in Security tab
640+
- ⚡ **Parallel Execution**: Runs simultaneously with main CI (faster)
641+
- 🔍 **Comprehensive**: Covers secrets, code vulnerabilities, and dependencies
642+
643+
**Note**: Both workflows must pass for PRs to be mergeable. See
644+
`docs/security-ci-review.md` for detailed integration guidance.
645+
604646
### Markdown Configuration
605647

606648
Markdown linting rules are configured in `.pymarkdown.json`:
@@ -653,10 +695,13 @@ This framework is designed with security as the highest priority:
653695
| Security Feature | Status | Description |
654696
|-----------------|--------|-------------|
655697
| 🔐 Secrets Protection | ✅ Active | Comprehensive `.gitignore` prevents accidental secret commits |
656-
| 🔍 Automated Detection | ✅ Active | Pre-commit hooks detect secrets via detect-secrets.sh |
698+
| 🔍 Pre-commit Detection | ✅ Active | Pre-commit hooks detect secrets via `detect-secrets.sh` |
699+
| 🔒 CI Secret Scanning | ✅ Active | Gitleaks scans full git history in CI (defense in depth) |
700+
| 🛡️ SAST Scanning | ✅ Active | Semgrep performs static security analysis (OWASP, security audit) |
701+
| 📦 Dependency Scanning | ✅ Active | OSV-Scanner checks dependencies for known vulnerabilities |
657702
| 🛡️ Least Privilege | ✅ Active | All scripts should use least privilege principles |
658703
| ✅ Input Validation | ✅ Active | All inputs should be treated as untrusted |
659-
| 📋 Audit Trail | ✅ Active | Pre-commit logs provide an audit trail of quality checks |
704+
| 📋 Audit Trail | ✅ Active | Pre-commit logs and CI artifacts provide audit trails |
660705

661706
### Security Best Practices
662707

‎docs/security-ci-review.md‎

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
1+
# Security CI Workflow Review and Integration Guide
2+
3+
## 📋 Review Summary
4+
5+
Your `security-ci.yml` workflow is **well-structured** and adds valuable security scanning
6+
capabilities. This document provides feedback, improvements, and integration recommendations.
7+
8+
---
9+
10+
## ✅ What's Good
11+
12+
1. **Comprehensive Coverage**: Three complementary security tools:
13+
- **Gitleaks**: Secret scanning (complements `detect-secrets.sh`)
14+
- **Semgrep**: SAST with security audit and OWASP Top 10 rules
15+
- **OSV-Scanner**: Dependency vulnerability scanning
16+
17+
2. **Proper Permissions**: `security-events: write` enables SARIF uploads to GitHub Security tab
18+
19+
3. **Appropriate Triggers**: Runs on PRs, pushes to main, and manual dispatch
20+
21+
4. **Git History Scanning**: `fetch-depth: 0` allows Gitleaks to scan full history
22+
23+
---
24+
25+
## 🔧 Improvements Made
26+
27+
### 1. **Consistent Naming and Formatting**
28+
29+
- Changed workflow name to "Security CI" (matches "CI" naming pattern)
30+
- Added consistent step names ("Checkout" instead of just using action)
31+
- Improved YAML formatting consistency
32+
33+
### 2. **OSV-Scanner Enhancements**
34+
35+
- Added JSON output format for better artifact handling
36+
- Added artifact upload for scan results (7-day retention)
37+
- Changed job name from `osv` to `osv-scanner` for clarity
38+
39+
### 3. **SARIF Upload Reliability**
40+
41+
- Added `if: always()` to SARIF upload to ensure results are uploaded even if Semgrep finds issues
42+
43+
---
44+
45+
## 🔄 Integration Options
46+
47+
You have **three integration approaches**:
48+
49+
### Option 1: Parallel Execution (Recommended) ⭐
50+
51+
**Keep workflows separate** - they run in parallel automatically.
52+
53+
**Pros**:
54+
- ✅ Security checks don't block code quality checks
55+
- ✅ Faster overall CI time (parallel execution)
56+
- ✅ Clear separation of concerns
57+
- ✅ Easy to disable security checks independently if needed
58+
- ✅ Matches GitHub's recommended pattern
59+
60+
**Cons**:
61+
- ⚠️ Two separate workflow statuses in PR checks
62+
63+
**Implementation**: No changes needed - workflows already run in parallel!
64+
65+
**Status Checks**: PRs will show:
66+
- ✅ CI (pre-commit + tests)
67+
- ✅ Security CI (gitleaks + semgrep + osv-scanner)
68+
69+
---
70+
71+
### Option 2: Sequential Integration
72+
73+
Make security checks a **dependency** of the main CI workflow.
74+
75+
**Pros**:
76+
- ✅ Single workflow status in PR
77+
- ✅ Security checks run before tests (fail fast)
78+
79+
**Cons**:
80+
- ⚠️ Slower overall CI time (sequential execution)
81+
- ⚠️ Security failures block tests
82+
- ⚠️ More complex workflow structure
83+
84+
**Implementation**: Modify `ci.yaml`:
85+
86+
```yaml
87+
jobs:
88+
security-checks:
89+
name: Security Checks
90+
runs-on: ubuntu-latest
91+
steps:
92+
- uses: actions/checkout@v4
93+
# ... (copy security jobs here or use workflow_call)
94+
95+
pre-commit:
96+
name: Pre-commit Checks
97+
runs-on: ubuntu-latest
98+
needs: [security-checks] # Add dependency
99+
# ... rest of job
100+
```
101+
102+
---
103+
104+
### Option 3: Unified Workflow
105+
106+
Merge both workflows into a single `ci.yaml` file.
107+
108+
**Pros**:
109+
- ✅ Single workflow file to maintain
110+
- ✅ Single status check in PR
111+
112+
**Cons**:
113+
- ⚠️ Larger, more complex workflow file
114+
- ⚠️ Less modular
115+
- ⚠️ Harder to run security checks independently
116+
117+
**Implementation**: Copy all security jobs into `ci.yaml` and remove `security-ci.yml`.
118+
119+
---
120+
121+
## 🎯 Recommendation: Option 1 (Parallel Execution)
122+
123+
**Why**:
124+
1. **Best Practice**: GitHub recommends separate workflows for different concerns
125+
2. **Performance**: Parallel execution is faster
126+
3. **Flexibility**: Can run security checks independently via `workflow_dispatch`
127+
4. **Clarity**: Clear separation between code quality and security
128+
5. **No Changes Needed**: Your current setup already works this way!
129+
130+
---
131+
132+
## 🔍 How It Works Together
133+
134+
### Current Setup (Parallel Execution)
135+
136+
```text
137+
Pull Request / Push
138+
│
139+
├─→ CI Workflow (ci.yaml)
140+
│ ├─→ pre-commit job
141+
│ └─→ tests job (depends on pre-commit)
142+
│
143+
└─→ Security CI Workflow (security-ci.yml)
144+
├─→ gitleaks job
145+
├─→ semgrep job
146+
└─→ osv-scanner job
147+
```
148+
149+
**Both workflows run simultaneously**, and the PR requires both to pass.
150+
151+
### Secret Detection Overlap
152+
153+
You have **two layers** of secret detection:
154+
155+
1. **Pre-commit (`detect-secrets.sh`)**: Fast, local, prevents secrets from being committed
156+
2. **CI (Gitleaks)**: Comprehensive, scans full git history, catches anything that slipped through
157+
158+
**This is intentional and recommended** - defense in depth!
159+
160+
---
161+
162+
## 📊 Workflow Comparison
163+
164+
| Aspect | Current Setup (Parallel) | Sequential | Unified |
165+
|--------|-------------------------|------------|---------|
166+
| **CI Time** | ⚡ Fastest (parallel) | 🐌 Slower (sequential) | 🐌 Slower (sequential) |
167+
| **Status Checks** | 2 separate | 1 combined | 1 combined |
168+
| **Maintainability** | ✅ High | ⚠️ Medium | ⚠️ Lower |
169+
| **Flexibility** | ✅ High | ⚠️ Medium | ⚠️ Lower |
170+
| **Best Practice** | ✅ Yes | ⚠️ Acceptable | ⚠️ Less ideal |
171+
172+
---
173+
174+
## 🚀 Next Steps
175+
176+
1. **Keep the workflows separate** (Option 1) - no changes needed!
177+
2. **Test the workflow** by creating a test PR
178+
3. **Monitor results** in the GitHub Security tab (Semgrep SARIF uploads)
179+
4. **Review OSV-Scanner results** in workflow artifacts
180+
181+
---
182+
183+
## 📝 Documentation Updates Needed
184+
185+
Consider updating:
186+
187+
1. **README.md**: Add a section about the Security CI workflow
188+
2. **CONTEXT.md**: Document that security scanning runs in CI (complements pre-commit)
189+
3. **ci-and-precommit.md**: Mention the security CI workflow
190+
191+
---
192+
193+
## 🔒 Security Posture
194+
195+
With this workflow, you now have:
196+
197+
| Layer | Tool | Purpose |
198+
|-------|------|---------|
199+
| **Pre-commit** | `detect-secrets.sh` | Fast local secret detection |
200+
| **CI - Secrets** | Gitleaks | Comprehensive secret scanning (full history) |
201+
| **CI - SAST** | Semgrep | Static code analysis (security audit, OWASP) |
202+
| **CI - Dependencies** | OSV-Scanner | Dependency vulnerability scanning |
203+
204+
**This is a robust, defense-in-depth security approach!** 🛡️

0 commit comments

Comments
 (0)