fix: add allowlist normalization and validation in permissions repair process
This commit is contained in:
GitHub Actions
7
.github/workflows/codeql.yml
vendored
7
.github/workflows/codeql.yml
vendored
@@ -2,7 +2,7 @@ name: CodeQL - Analyze
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
branches: [main, nightly]
|
||||
branches: [main, nightly, development]
|
||||
push:
|
||||
branches: [main, nightly, development]
|
||||
workflow_dispatch:
|
||||
@@ -42,10 +42,15 @@ jobs:
|
||||
with:
|
||||
ref: ${{ github.sha }}
|
||||
|
||||
- name: Verify CodeQL parity guard
|
||||
if: matrix.language == 'go'
|
||||
run: bash scripts/ci/check-codeql-parity.sh
|
||||
|
||||
- name: Initialize CodeQL
|
||||
uses: github/codeql-action/init@9e907b5e64f6b83e7804b09294d44122997950d6 # v4
|
||||
with:
|
||||
languages: ${{ matrix.language }}
|
||||
queries: security-and-quality
|
||||
# Use CodeQL config to exclude documented false positives
|
||||
# Go: Excludes go/request-forgery for url_testing.go (has 4-layer SSRF defense)
|
||||
# See: .github/codeql/codeql-config.yml for full justification
|
||||
|
||||
@@ -168,6 +168,14 @@ repos:
|
||||
verbose: true
|
||||
stages: [manual] # Only runs after CodeQL scans
|
||||
|
||||
- id: codeql-parity-check
|
||||
name: CodeQL Suite/Trigger Parity Guard (Manual)
|
||||
entry: scripts/ci/check-codeql-parity.sh
|
||||
language: script
|
||||
pass_filenames: false
|
||||
verbose: true
|
||||
stages: [manual]
|
||||
|
||||
- id: gorm-security-scan
|
||||
name: GORM Security Scanner (Manual)
|
||||
entry: scripts/pre-commit-hooks/gorm-security-check.sh
|
||||
|
||||
2
.vscode/tasks.json
vendored
2
.vscode/tasks.json
vendored
@@ -459,7 +459,7 @@
|
||||
{
|
||||
"label": "Security: CodeQL Go Scan (CI-Aligned) [~60s]",
|
||||
"type": "shell",
|
||||
"command": "rm -rf codeql-db-go && codeql database create codeql-db-go --language=go --source-root=backend --codescanning-config=.github/codeql/codeql-config.yml --overwrite --threads=0 && codeql database analyze codeql-db-go --additional-packs=codeql-custom-queries-go --format=sarif-latest --output=codeql-results-go.sarif --sarif-add-baseline-file-info --threads=0",
|
||||
"command": "bash scripts/pre-commit-hooks/codeql-go-scan.sh",
|
||||
"group": "test",
|
||||
"problemMatcher": []
|
||||
},
|
||||
|
||||
@@ -135,6 +135,16 @@ func (h *SystemPermissionsHandler) repairPath(rawPath string, groupMode bool, al
|
||||
}
|
||||
}
|
||||
|
||||
normalizedAllowlist := normalizeAllowlist(allowlist)
|
||||
if !isWithinAllowlist(cleanPath, normalizedAllowlist) {
|
||||
return permissionsRepairResult{
|
||||
Path: cleanPath,
|
||||
Status: "error",
|
||||
ErrorCode: "permissions_outside_allowlist",
|
||||
Message: "path outside allowlist",
|
||||
}
|
||||
}
|
||||
|
||||
info, err := os.Lstat(cleanPath)
|
||||
if err != nil {
|
||||
if os.IsNotExist(err) {
|
||||
@@ -198,7 +208,7 @@ func (h *SystemPermissionsHandler) repairPath(rawPath string, groupMode bool, al
|
||||
}
|
||||
}
|
||||
|
||||
if !isWithinAllowlist(resolved, allowlist) {
|
||||
if !isWithinAllowlist(resolved, normalizedAllowlist) {
|
||||
return permissionsRepairResult{
|
||||
Path: cleanPath,
|
||||
Status: "error",
|
||||
@@ -358,6 +368,17 @@ func containsParentReference(clean string) bool {
|
||||
return strings.HasSuffix(clean, string(os.PathSeparator)+"..")
|
||||
}
|
||||
|
||||
func normalizeAllowlist(allowlist []string) []string {
|
||||
normalized := make([]string, 0, len(allowlist))
|
||||
for _, root := range allowlist {
|
||||
if root == "" {
|
||||
continue
|
||||
}
|
||||
normalized = append(normalized, filepath.Clean(root))
|
||||
}
|
||||
return normalized
|
||||
}
|
||||
|
||||
func pathHasSymlink(path string) (bool, error) {
|
||||
clean := filepath.Clean(path)
|
||||
parts := strings.Split(clean, string(os.PathSeparator))
|
||||
|
||||
@@ -487,6 +487,14 @@ func TestSystemPermissionsHandler_RepairPath_Branches(t *testing.T) {
|
||||
require.Equal(t, "permissions_outside_allowlist", result.ErrorCode)
|
||||
})
|
||||
|
||||
t.Run("outside allowlist rejected before stat for missing path", func(t *testing.T) {
|
||||
outsideMissing := filepath.Join(t.TempDir(), "missing.txt")
|
||||
|
||||
result := h.repairPath(outsideMissing, false, allowlist)
|
||||
require.Equal(t, "error", result.Status)
|
||||
require.Equal(t, "permissions_outside_allowlist", result.ErrorCode)
|
||||
})
|
||||
|
||||
t.Run("unsupported type rejected", func(t *testing.T) {
|
||||
fifoPath := filepath.Join(allowRoot, "fifo")
|
||||
require.NoError(t, syscall.Mkfifo(fifoPath, 0o600))
|
||||
@@ -559,7 +567,7 @@ func TestSystemPermissionsHandler_RepairPath_LstatInvalidArgument(t *testing.T)
|
||||
|
||||
result := h.repairPath("/tmp/\x00invalid", false, []string{allowRoot})
|
||||
require.Equal(t, "error", result.Status)
|
||||
require.Equal(t, "permissions_repair_failed", result.ErrorCode)
|
||||
require.Equal(t, "permissions_outside_allowlist", result.ErrorCode)
|
||||
}
|
||||
|
||||
func TestSystemPermissionsHandler_RepairPath_RepairedBranch(t *testing.T) {
|
||||
@@ -590,3 +598,8 @@ func TestSystemPermissionsHandler_NormalizePath_ParentRefBranches(t *testing.T)
|
||||
require.Equal(t, "/etc", clean)
|
||||
require.Empty(t, code)
|
||||
}
|
||||
|
||||
func TestSystemPermissionsHandler_NormalizeAllowlist(t *testing.T) {
|
||||
allowlist := normalizeAllowlist([]string{"", "/tmp/data/..", "/var/log/charon"})
|
||||
require.Equal(t, []string{"/tmp", "/var/log/charon"}, allowlist)
|
||||
}
|
||||
|
||||
@@ -1,572 +1,135 @@
|
||||
## Local Pre-CI Patch Report (Single Scope)
|
||||
|
||||
Date: 2026-02-17
|
||||
Scope: Add a local pre-CI patch report to Definition of Done (DoD) unit-testing flow for both backend and frontend.
|
||||
|
||||
## 1) Objective
|
||||
|
||||
Add one executable local workflow that computes patch coverage from current branch changes and publishes a consolidated report before CI runs.
|
||||
|
||||
The report must consume backend and frontend coverage inputs, use `origin/main...HEAD` as the patch baseline, and produce human-readable and machine-readable artifacts in `test-results/`.
|
||||
|
||||
## 2) In Scope / Out of Scope
|
||||
|
||||
### In Scope
|
||||
|
||||
- Local patch report generation.
|
||||
- Backend + frontend DoD unit-testing integration.
|
||||
- VS Code task wiring for repeatable local execution.
|
||||
- Non-blocking warning policy for initial rollout.
|
||||
|
||||
### Out of Scope
|
||||
|
||||
- CI gate changes.
|
||||
- Encryption-key or unrelated reliability/security remediation.
|
||||
- Historical Codecov placeholder gates and unrelated patch-closure matrices.
|
||||
|
||||
## 3) Required Inputs and Baseline
|
||||
|
||||
### Coverage Inputs
|
||||
|
||||
- Backend coverage profile: `backend/coverage.txt`
|
||||
- Frontend coverage profile: `frontend/coverage/lcov.info`
|
||||
|
||||
### Diff Baseline
|
||||
|
||||
- Git diff range: `origin/main...HEAD`
|
||||
|
||||
### Preconditions
|
||||
|
||||
- `origin/main` is fetchable locally.
|
||||
- Backend and frontend coverage artifacts exist before report generation.
|
||||
|
||||
## 4) Required Output Artifacts
|
||||
|
||||
- Markdown report: `test-results/local-patch-report.md`
|
||||
- JSON report: `test-results/local-patch-report.json`
|
||||
|
||||
Both artifacts are mandatory per run. Missing either artifact is a failed local report run.
|
||||
|
||||
## 5) Initial Policy (Rollout)
|
||||
|
||||
### Initial Policy (Non-Blocking)
|
||||
|
||||
- Local patch report does not fail DoD on low patch coverage during initial rollout.
|
||||
- Local runner emits warnings (stdout + markdown/json status fields) when thresholds are not met.
|
||||
- DoD requires the report to run and artifacts to exist, even in warning mode.
|
||||
- Execution and final merge checks in this plan follow this same warn-mode policy during rollout.
|
||||
|
||||
### Threshold Defaults and Source Precedence
|
||||
|
||||
- Coverage thresholds are resolved with this precedence:
|
||||
1. Environment variables (highest precedence)
|
||||
2. Built-in defaults (fallback)
|
||||
- Threshold environment variables:
|
||||
- `CHARON_OVERALL_PATCH_COVERAGE_MIN`
|
||||
- `CHARON_BACKEND_PATCH_COVERAGE_MIN`
|
||||
- `CHARON_FRONTEND_PATCH_COVERAGE_MIN`
|
||||
- Built-in defaults for this rollout:
|
||||
- Overall patch coverage minimum: `90`
|
||||
- Backend patch coverage minimum: `85`
|
||||
- Frontend patch coverage minimum: `85`
|
||||
- Parsing/validation:
|
||||
- Values must be numeric percentages in `[0, 100]`.
|
||||
- Invalid env values are ignored with a warning, and the corresponding default is used.
|
||||
|
||||
### Future Policy (Optional Hard Gate)
|
||||
|
||||
- Optional future switch to hard gate (non-zero exit on threshold breach).
|
||||
- Gate behavior is controlled by a dedicated flag/env (to be added during implementation).
|
||||
- Hard-gate enablement is explicitly deferred and not part of this rollout.
|
||||
|
||||
## 6) Technical Specification
|
||||
|
||||
### 6.1 Script
|
||||
|
||||
Implement a new local report script:
|
||||
|
||||
- Path: `scripts/local-patch-report.sh`
|
||||
- Responsibilities:
|
||||
1. Validate required inputs exist (`backend/coverage.txt`, `frontend/coverage/lcov.info`).
|
||||
2. Resolve patch files/lines from `origin/main...HEAD`.
|
||||
3. Correlate changed lines with backend/frontend coverage data.
|
||||
4. Compute patch summary by component and overall.
|
||||
5. Resolve thresholds using env-var-first precedence, then defaults (`90/85/85`).
|
||||
6. Evaluate statuses against resolved thresholds:
|
||||
- `overall.status=pass` when `overall.patch_coverage_pct >= overall_threshold`, else `warn`.
|
||||
- `backend.status=pass` when `backend.patch_coverage_pct >= backend_threshold`, else `warn`.
|
||||
- `frontend.status=pass` when `frontend.patch_coverage_pct >= frontend_threshold`, else `warn`.
|
||||
7. Emit warning status when any scope is below its resolved threshold.
|
||||
8. Write required outputs:
|
||||
- `test-results/local-patch-report.md`
|
||||
- `test-results/local-patch-report.json`
|
||||
|
||||
### 6.2 Report Contract
|
||||
|
||||
Minimum JSON fields:
|
||||
|
||||
- `baseline`: `origin/main...HEAD`
|
||||
- `generated_at`
|
||||
- `mode`: `warn` (initial rollout)
|
||||
- `thresholds`:
|
||||
- `overall_patch_coverage_min`
|
||||
- `backend_patch_coverage_min`
|
||||
- `frontend_patch_coverage_min`
|
||||
- `threshold_sources`:
|
||||
- `overall` (`env` | `default`)
|
||||
- `backend` (`env` | `default`)
|
||||
- `frontend` (`env` | `default`)
|
||||
- `overall`:
|
||||
- `changed_lines`
|
||||
- `covered_lines`
|
||||
- `patch_coverage_pct`
|
||||
- `status` (`pass` | `warn`)
|
||||
- `backend` and `frontend` objects with same coverage counters and status
|
||||
- `files_needing_coverage` (required array for execution baselines), where each item includes at minimum:
|
||||
- `path`
|
||||
- `uncovered_changed_lines`
|
||||
- `patch_coverage_pct`
|
||||
- `artifacts` with emitted file paths
|
||||
|
||||
Minimum Markdown sections:
|
||||
|
||||
- Run metadata (timestamp, baseline)
|
||||
- Input paths used
|
||||
- Resolved thresholds and their sources (env/default)
|
||||
- Coverage summary table (overall/backend/frontend)
|
||||
- Warning section (if any)
|
||||
- Artifact paths
|
||||
|
||||
### 6.3 Task Wiring
|
||||
|
||||
Add VS Code task entries in `.vscode/tasks.json`:
|
||||
|
||||
1. `Test: Local Patch Report`
|
||||
- Runs report generation script only.
|
||||
2. `Test: Backend DoD + Local Patch Report`
|
||||
- Runs backend unit test coverage flow, then local patch report.
|
||||
3. `Test: Frontend DoD + Local Patch Report`
|
||||
- Runs frontend unit test coverage flow, then local patch report.
|
||||
4. `Test: Full DoD Unit + Local Patch Report`
|
||||
- Runs backend + frontend unit coverage flows, then local patch report.
|
||||
|
||||
Task behavior:
|
||||
|
||||
- Reuse existing coverage scripts/tasks where available.
|
||||
- Keep command order deterministic: coverage generation first, patch report second.
|
||||
|
||||
## 7) Implementation Tasks
|
||||
|
||||
### Phase 1 — Script Foundation
|
||||
|
||||
- [ ] Create `scripts/local-patch-report.sh`.
|
||||
- [ ] Add input validation + clear error messages.
|
||||
- [ ] Add diff parsing for `origin/main...HEAD`.
|
||||
|
||||
### Phase 2 — Coverage Correlation
|
||||
|
||||
- [ ] Parse backend `coverage.txt` and map covered lines.
|
||||
- [ ] Parse frontend `coverage/lcov.info` and map covered lines.
|
||||
- [ ] Compute per-scope and overall patch coverage counters.
|
||||
|
||||
### Phase 3 — Artifact Emission
|
||||
|
||||
- [ ] Generate `test-results/local-patch-report.json` with required schema.
|
||||
- [ ] Generate `test-results/local-patch-report.md` with summary + warnings.
|
||||
- [ ] Ensure `test-results/` creation if missing.
|
||||
|
||||
### Phase 4 — Task Wiring
|
||||
|
||||
- [ ] Add `Test: Local Patch Report` to `.vscode/tasks.json`.
|
||||
- [ ] Add backend/frontend/full DoD task variants with report execution.
|
||||
- [ ] Verify tasks run successfully from workspace root.
|
||||
|
||||
### Phase 5 — Documentation Alignment
|
||||
|
||||
- [ ] Update DoD references in applicable docs/instructions only where this local report is now required.
|
||||
- [ ] Remove stale references to unrelated placeholder gates in active plan context.
|
||||
|
||||
## 8) Validation Commands
|
||||
|
||||
Run from repository root unless noted.
|
||||
|
||||
1. Generate backend coverage input:
|
||||
|
||||
```bash
|
||||
cd backend && go test ./... -coverprofile=coverage.txt
|
||||
```
|
||||
|
||||
2. Generate frontend coverage input:
|
||||
|
||||
```bash
|
||||
cd frontend && npm run test:coverage
|
||||
```
|
||||
|
||||
3. Generate local patch report directly:
|
||||
|
||||
```bash
|
||||
./scripts/local-patch-report.sh
|
||||
```
|
||||
|
||||
4. Generate local patch report via task:
|
||||
|
||||
```bash
|
||||
# VS Code task: Test: Local Patch Report
|
||||
```
|
||||
|
||||
5. Validate artifacts exist:
|
||||
|
||||
```bash
|
||||
test -f test-results/local-patch-report.md
|
||||
test -f test-results/local-patch-report.json
|
||||
```
|
||||
|
||||
6. Validate baseline recorded in JSON:
|
||||
|
||||
```bash
|
||||
jq -r '.baseline' test-results/local-patch-report.json
|
||||
# expected: origin/main...HEAD
|
||||
```
|
||||
|
||||
## 9) Acceptance Criteria
|
||||
|
||||
- [ ] Plan remains single-scope: local pre-CI patch report for DoD unit testing only.
|
||||
- [ ] Inputs are explicit and used:
|
||||
- [ ] `backend/coverage.txt`
|
||||
- [ ] `frontend/coverage/lcov.info`
|
||||
- [ ] `origin/main...HEAD`
|
||||
- [ ] Outputs are generated on every successful run:
|
||||
- [ ] `test-results/local-patch-report.md`
|
||||
- [ ] `test-results/local-patch-report.json`
|
||||
- [ ] Initial policy is non-blocking warning mode.
|
||||
- [ ] Default thresholds are explicit:
|
||||
- [ ] Overall patch coverage: `90`
|
||||
- [ ] Backend patch coverage: `85`
|
||||
- [ ] Frontend patch coverage: `85`
|
||||
- [ ] Threshold source precedence is explicit: env vars first, then defaults.
|
||||
- [ ] Future hard-gate mode is documented as optional and deferred.
|
||||
- [ ] Concrete script + task wiring tasks are present and executable.
|
||||
- [ ] Validation commands are present and reproducible.
|
||||
- [ ] Stale unrelated placeholder gates are removed from this active spec.
|
||||
|
||||
## 10) Concrete Execution Plan — Patch Gap Closure (PR Merge Objective)
|
||||
|
||||
Single-scope objective: close current patch gaps for this PR merge by adding targeted tests and iterating local patch reports until changed-line coverage is merge-ready under DoD.
|
||||
|
||||
### Authoritative Gap Baseline (2026-02-17)
|
||||
|
||||
Use this list as the only planning baseline for this execution cycle:
|
||||
|
||||
- `backend/cmd/localpatchreport/main.go`: 0%, 200 uncovered changed lines, ranges `46-59`, `61-73`, `75-79`, `81-85`, `87-96`, `98-123`, `125-156`, `158-165`, `167-172`, `175-179`, `182-187`, `190-198`, `201-207`, `210-219`, `222-254`, `257-264`, `267-269`
|
||||
- `frontend/src/pages/UsersPage.tsx`: 30.8%, 9 uncovered (`152-160`)
|
||||
- `frontend/src/pages/CrowdSecConfig.tsx`: 36.8%, 12 uncovered (`975-977`, `1220`, `1248-1249`, `1281-1282`, `1316`, `1324-1325`, `1335`)
|
||||
- `frontend/src/pages/DNSProviders.tsx`: 70.6%, 10 uncovered
|
||||
- `frontend/src/pages/AuditLogs.tsx`: 75.0%, 1 uncovered
|
||||
- `frontend/src/components/ProxyHostForm.tsx`: 75.5%, 12 uncovered
|
||||
- `backend/internal/api/middleware/auth.go`: 86.4%, 3 uncovered
|
||||
- `frontend/src/pages/Notifications.tsx`: 88.9%, 3 uncovered
|
||||
- `backend/internal/cerberus/rate_limit.go`: 91.9%, 12 uncovered
|
||||
|
||||
### DoD Entry Gate (Mandatory Before Phase 1)
|
||||
|
||||
All execution phases are blocked until this gate is completed in order:
|
||||
|
||||
1) E2E first:
|
||||
|
||||
```bash
|
||||
cd /projects/Charon && npx playwright test --project=firefox
|
||||
```
|
||||
|
||||
2) Local patch preflight (baseline refresh trigger):
|
||||
|
||||
```bash
|
||||
cd /projects/Charon && bash scripts/local-patch-report.sh
|
||||
```
|
||||
|
||||
3) Baseline refresh checkpoint (must pass before phase execution):
|
||||
|
||||
```bash
|
||||
cd /projects/Charon && jq -r '.files_needing_coverage[].path' test-results/local-patch-report.json | sort > /tmp/charon-baseline-files.txt
|
||||
cd /projects/Charon && while read -r f; do git diff --name-only origin/main...HEAD -- "$f" | grep -qx "$f" || echo "baseline file missing from current diff: $f"; done < /tmp/charon-baseline-files.txt
|
||||
```
|
||||
|
||||
4) If checkpoint output is non-empty, refresh this baseline list to match the latest `test-results/local-patch-report.json` before starting Phase 1.
|
||||
|
||||
### Ordered Phases (Highest Impact First)
|
||||
|
||||
#### Phase 1 — Backend Local Patch Report CLI (Highest Delta)
|
||||
|
||||
Targets:
|
||||
- `backend/cmd/localpatchreport/main.go` (all listed uncovered ranges)
|
||||
|
||||
Suggested test file:
|
||||
- `backend/cmd/localpatchreport/main_test.go`
|
||||
|
||||
Test focus:
|
||||
- argument parsing and mode selection
|
||||
- coverage input validation paths
|
||||
- baseline/diff resolution flow
|
||||
- report generation branches (markdown/json)
|
||||
- warning/error branches for missing inputs and malformed coverage
|
||||
|
||||
Pass criteria:
|
||||
- maximize reduction of uncovered changed lines in `backend/cmd/localpatchreport/main.go` from the `200` baseline, with priority on highest-impact uncovered ranges and no new uncovered changed lines introduced
|
||||
- backend targeted test command passes
|
||||
|
||||
Targeted test command:
|
||||
|
||||
```bash
|
||||
cd /projects/Charon/backend && go test ./cmd/localpatchreport -coverprofile=coverage.txt
|
||||
```
|
||||
|
||||
#### Phase 2 — Frontend Lowest-Coverage, Highest-Uncovered Pages
|
||||
|
||||
Targets:
|
||||
- `frontend/src/pages/CrowdSecConfig.tsx` (`975-977`, `1220`, `1248-1249`, `1281-1282`, `1316`, `1324-1325`, `1335`)
|
||||
- `frontend/src/pages/UsersPage.tsx` (`152-160`)
|
||||
- `frontend/src/pages/DNSProviders.tsx` (10 uncovered changed lines)
|
||||
|
||||
Suggested test files:
|
||||
- `frontend/src/pages/__tests__/CrowdSecConfig.patch-gap.test.tsx`
|
||||
- `frontend/src/pages/__tests__/UsersPage.patch-gap.test.tsx`
|
||||
- `frontend/src/pages/__tests__/DNSProviders.patch-gap.test.tsx`
|
||||
|
||||
Test focus:
|
||||
- branch/error-state rendering tied to uncovered lines
|
||||
- conditional action handlers and callback guards
|
||||
- edge-case interaction states not hit by existing tests
|
||||
|
||||
Pass criteria:
|
||||
- maximize reduction of changed-line gaps for the three targets, prioritize highest-impact uncovered lines first, and avoid introducing new uncovered changed lines
|
||||
- frontend targeted test command passes
|
||||
|
||||
Targeted test command:
|
||||
|
||||
```bash
|
||||
cd /projects/Charon/frontend && npm run test:coverage -- src/pages/__tests__/CrowdSecConfig.patch-gap.test.tsx src/pages/__tests__/UsersPage.patch-gap.test.tsx src/pages/__tests__/DNSProviders.patch-gap.test.tsx
|
||||
```
|
||||
|
||||
#### Phase 3 — Backend Residual Middleware/Security Gaps
|
||||
|
||||
Targets:
|
||||
- `backend/internal/api/middleware/auth.go` (3 uncovered changed lines)
|
||||
- `backend/internal/cerberus/rate_limit.go` (12 uncovered changed lines)
|
||||
|
||||
Suggested test targets/files:
|
||||
- extend `backend/internal/api/middleware/auth_test.go`
|
||||
- extend `backend/internal/cerberus/rate_limit_test.go`
|
||||
|
||||
Test focus:
|
||||
- auth middleware edge branches (token/context failure paths)
|
||||
- rate-limit boundary and deny/allow branch coverage
|
||||
|
||||
Pass criteria:
|
||||
- maximize reduction of changed-line gaps for both backend files, prioritize highest-impact uncovered lines first, and avoid introducing new uncovered changed lines
|
||||
- backend targeted test command passes
|
||||
|
||||
Targeted test command:
|
||||
|
||||
```bash
|
||||
cd /projects/Charon/backend && go test ./internal/api/middleware ./internal/cerberus -coverprofile=coverage.txt
|
||||
```
|
||||
|
||||
#### Phase 4 — Frontend Component + Residual Page Gaps
|
||||
|
||||
Targets:
|
||||
- `frontend/src/components/ProxyHostForm.tsx` (12 uncovered changed lines)
|
||||
- `frontend/src/pages/AuditLogs.tsx` (1 uncovered changed line)
|
||||
- `frontend/src/pages/Notifications.tsx` (3 uncovered changed lines)
|
||||
|
||||
Suggested test files:
|
||||
- `frontend/src/components/__tests__/ProxyHostForm.patch-gap.test.tsx`
|
||||
- `frontend/src/pages/__tests__/AuditLogs.patch-gap.test.tsx`
|
||||
- `frontend/src/pages/__tests__/Notifications.patch-gap.test.tsx`
|
||||
|
||||
Test focus:
|
||||
- form branch paths and validation fallbacks
|
||||
- single-line residual branch in audit logs
|
||||
- notification branch handling for low-frequency states
|
||||
|
||||
Pass criteria:
|
||||
- maximize reduction of changed-line gaps for all three targets, prioritize highest-impact uncovered lines first, and avoid introducing new uncovered changed lines
|
||||
- frontend targeted test command passes
|
||||
|
||||
Targeted test command:
|
||||
|
||||
```bash
|
||||
cd /projects/Charon/frontend && npm run test:coverage -- src/components/__tests__/ProxyHostForm.patch-gap.test.tsx src/pages/__tests__/AuditLogs.patch-gap.test.tsx src/pages/__tests__/Notifications.patch-gap.test.tsx
|
||||
```
|
||||
|
||||
### Execution Commands
|
||||
|
||||
Run from repository root unless stated otherwise.
|
||||
|
||||
1) Backend coverage:
|
||||
|
||||
```bash
|
||||
cd backend && go test ./... -coverprofile=coverage.txt
|
||||
```
|
||||
|
||||
2) Frontend coverage:
|
||||
|
||||
```bash
|
||||
cd frontend && npm run test:coverage
|
||||
```
|
||||
|
||||
3) Local patch report iteration:
|
||||
|
||||
```bash
|
||||
bash scripts/local-patch-report.sh
|
||||
```
|
||||
|
||||
4) Iteration loop (repeat until all target gaps are closed):
|
||||
|
||||
```bash
|
||||
cd backend && go test ./... -coverprofile=coverage.txt
|
||||
cd /projects/Charon/frontend && npm run test:coverage
|
||||
cd /projects/Charon && bash scripts/local-patch-report.sh
|
||||
```
|
||||
|
||||
### Phase Completion Checks
|
||||
|
||||
- After each phase, rerun `bash scripts/local-patch-report.sh` and confirm that only the next planned target set remains uncovered.
|
||||
- Do not advance phases when a phase target still shows uncovered changed lines.
|
||||
|
||||
### Final Merge-Ready Gate (DoD-Aligned, Warn-Mode Rollout)
|
||||
|
||||
This PR is merge-ready only when all conditions are true:
|
||||
|
||||
- local patch report runs in warn mode and required artifacts are generated
|
||||
- practical merge objective: drive a significant reduction in authoritative baseline uncovered changed lines in this PR, prioritizing highest-impact files; `0` remains aspirational and is not a warn-mode merge blocker
|
||||
- required artifacts exist and are current:
|
||||
- `test-results/local-patch-report.md`
|
||||
- `test-results/local-patch-report.json`
|
||||
- backend and frontend coverage commands complete successfully
|
||||
- DoD checks remain satisfied (E2E first, local patch report preflight, required security/coverage/type/build validations)
|
||||
|
||||
---
|
||||
|
||||
## Flaky Test Stabilization Plan: `TestSettingsHandlerWave4_PatchConfig_SecurityReloadSuccessLogsPath` (2026-02-17)
|
||||
|
||||
### 1) Scope and Objective
|
||||
|
||||
Stabilize backend flake in `backend/internal/api/handlers/settings_wave4_test.go` for:
|
||||
|
||||
- `TestSettingsHandlerWave4_PatchConfig_SecurityReloadSuccessLogsPath`
|
||||
|
||||
Scope is limited to this flaky path and directly adjacent test/lifecycle hardening required to make behavior deterministic across CI contexts.
|
||||
|
||||
### 2) Investigation Findings (Root Cause)
|
||||
|
||||
Evidence from CI and local repro (`go test -race -count=20 -run 'TestSettingsHandlerWave4_UpdateSetting_ACLPathsPermissionErrors|TestSettingsHandlerWave4_PatchConfig_SecurityReloadSuccessLogsPath' ./internal/api/handlers`):
|
||||
|
||||
- Race is reported by Go race detector during execution of `TestSettingsHandlerWave4_PatchConfig_SecurityReloadSuccessLogsPath`.
|
||||
- Conflicting operations:
|
||||
- **Read path**: background goroutine from `services.NewSecurityService()` performing `db.Create()` in `persistAuditWithRetry()` / `processAuditEvents()`.
|
||||
- **Write path**: test cleanup removing GORM create callback (`db.Callback().Create().Remove(...)`) in `registerCreatePermissionDeniedHook` cleanup.
|
||||
- This race is triggered by preceding test `TestSettingsHandlerWave4_UpdateSetting_ACLPathsPermissionErrors`, which creates a `SecurityService` (spawns goroutine) and does not shut it down before callback cleanup mutates callback registry.
|
||||
|
||||
Primary cause is **shared mutable callback registry + still-running background audit goroutine** (order-dependent teardown), not business logic in `PatchConfig` itself.
|
||||
|
||||
### 3) Dependency Map (Files and Symbols)
|
||||
|
||||
#### Test path
|
||||
|
||||
- `backend/internal/api/handlers/settings_wave4_test.go`
|
||||
- `TestSettingsHandlerWave4_PatchConfig_SecurityReloadSuccessLogsPath`
|
||||
- `TestSettingsHandlerWave4_UpdateSetting_ACLPathsPermissionErrors`
|
||||
- `registerCreatePermissionDeniedHook`
|
||||
- `setupSettingsWave3DB`
|
||||
|
||||
#### Handler/runtime path
|
||||
|
||||
- `backend/internal/api/handlers/settings_handler.go`
|
||||
- `PatchConfig`
|
||||
- `UpdateSetting`
|
||||
- `backend/internal/api/handlers/permission_helpers.go`
|
||||
- `respondPermissionError`
|
||||
- `logPermissionAudit`
|
||||
- `backend/internal/services/security_service.go`
|
||||
- `NewSecurityService`
|
||||
- `LogAudit`
|
||||
- `processAuditEvents`
|
||||
- `Close`
|
||||
- `Flush`
|
||||
|
||||
#### CI execution context
|
||||
|
||||
- `scripts/go-test-coverage.sh` (always runs backend tests with `-race`)
|
||||
- `.github/workflows/codecov-upload.yml` (uses `scripts/go-test-coverage.sh` for both push and PR)
|
||||
|
||||
### 4) Flake Vector Assessment
|
||||
|
||||
- **Timing/Goroutines**: High confidence root cause. Background audit goroutine outlives test branch and races with callback registry mutation.
|
||||
- **Shared state/global hooks**: High confidence root cause. GORM callback registry is mutable shared state per DB instance.
|
||||
- **Order dependence**: High confidence root cause. Preceding wave4 permission-error test influences subsequent test via asynchronous cleanup timing.
|
||||
- **DB locking/no-such-table noise**: Secondary contributor (observed `security_audits` missing logs), but not primary failure trigger.
|
||||
- **Env vars (PR vs push)**: Low confidence as root cause for this test; same script and `-race` path are used in both contexts.
|
||||
- **Log buffering**: Not a primary root cause; race detector output indicates memory race in callback internals.
|
||||
|
||||
### 5) Stabilization Strategy (Minimal and Deterministic)
|
||||
|
||||
#### Recommended approach
|
||||
|
||||
1. **Deterministic lifecycle shutdown for `SecurityService` in wave4 permission-error test**
|
||||
- In `TestSettingsHandlerWave4_UpdateSetting_ACLPathsPermissionErrors`, explicitly manage the service used for `h.SecuritySvc` and register teardown to flush/close it before callback removal side effects complete.
|
||||
- Ensure cleanup order prevents callback registry mutation while audit goroutine is still active.
|
||||
|
||||
2. **Reduce unnecessary async audit side effects in this wave4 path**
|
||||
- For tests that only assert HTTP permission error response (not audit persistence), avoid creating live async service when not required by assertion semantics.
|
||||
- Keep behavior coverage for response contract while eliminating unnecessary goroutine work in this flaky sequence.
|
||||
|
||||
3. **Harden test DB schema for adjacent audit paths**
|
||||
- In `setupSettingsWave3DB`, include `models.SecurityAudit` migration to remove noisy `no such table: security_audits` writes from concurrent worker paths.
|
||||
- This reduces background retry/noise and improves determinism under race mode.
|
||||
|
||||
4. **Guard callback hook helper usage**
|
||||
- Keep callback registration/removal confined to narrow tests and avoid overlap with asynchronous writers on same DB handle.
|
||||
- Maintain unique callback naming per test branch to prevent accidental collisions when future subtests are added.
|
||||
|
||||
### 6) EARS Requirements
|
||||
|
||||
- WHEN wave4 permission-error tests register temporary GORM callbacks, THE SYSTEM SHALL ensure all asynchronous `SecurityService` audit workers are fully stopped before callback removal occurs.
|
||||
- WHEN `TestSettingsHandlerWave4_PatchConfig_SecurityReloadSuccessLogsPath` runs with `-race`, THE SYSTEM SHALL complete without data race reports.
|
||||
- IF a test path uses `SecurityService.LogAudit`, THEN the test DB setup SHALL include required audit schema to avoid asynchronous write failures due to missing tables.
|
||||
- WHILE running backend coverage in CI contexts (push and PR), THE SYSTEM SHALL produce deterministic pass/fail outcomes for this test sequence.
|
||||
|
||||
### 7) Implementation Tasks (Single-Scope)
|
||||
|
||||
1. Update `backend/internal/api/handlers/settings_wave4_test.go`
|
||||
- Add explicit `SecurityService` lifecycle management in `TestSettingsHandlerWave4_UpdateSetting_ACLPathsPermissionErrors`.
|
||||
- Ensure teardown ordering is deterministic relative to callback cleanup.
|
||||
- Keep `TestSettingsHandlerWave4_PatchConfig_SecurityReloadSuccessLogsPath` assertions unchanged (status + reload/cache call counts).
|
||||
|
||||
2. Update `backend/internal/api/handlers/settings_wave3_test.go`
|
||||
- Extend `setupSettingsWave3DB` migrations to include `models.SecurityAudit`.
|
||||
|
||||
3. Validation
|
||||
- Targeted race test loop:
|
||||
- `cd backend && CHARON_ENCRYPTION_KEY="$(openssl rand -base64 32)" go test -race -count=50 -run 'TestSettingsHandlerWave4_UpdateSetting_ACLPathsPermissionErrors|TestSettingsHandlerWave4_PatchConfig_SecurityReloadSuccessLogsPath' ./internal/api/handlers`
|
||||
- Targeted package race pass:
|
||||
- `cd backend && CHARON_ENCRYPTION_KEY="$(openssl rand -base64 32)" go test -race -run 'TestSettingsHandlerWave4_' ./internal/api/handlers`
|
||||
- Standard backend CI-equivalent coverage command:
|
||||
- `bash scripts/go-test-coverage.sh`
|
||||
|
||||
### 8) PR Slicing Strategy
|
||||
|
||||
- **Decision**: Single PR (small, isolated, low blast radius).
|
||||
- **Trigger rationale**: Changes are constrained to wave4 settings tests and adjacent test helper DB schema.
|
||||
- **Slice PR-1**:
|
||||
- Scope: lifecycle/order hardening + helper schema migration only.
|
||||
- Files:
|
||||
- `backend/internal/api/handlers/settings_wave4_test.go`
|
||||
- `backend/internal/api/handlers/settings_wave3_test.go`
|
||||
- Validation gate: no race detector output in targeted loop; package tests stable under `-race`; no assertion behavior drift in target flaky test.
|
||||
- **Rollback**: Revert PR-1 if unintended changes appear in broader handlers suite; no production code path changes expected.
|
||||
|
||||
### 9) Acceptance Criteria
|
||||
|
||||
- `TestSettingsHandlerWave4_PatchConfig_SecurityReloadSuccessLogsPath` is stable under repeated `-race` runs.
|
||||
- No race detector warnings involving GORM callback compile/remove and `SecurityService` audit goroutine in this test sequence.
|
||||
- Test remains behaviorally equivalent (same API contract and assertions).
|
||||
- Scope remains limited to this flaky test sequence and adjacent stabilization only.
|
||||
## CodeQL Go Coverage RCA (2026-02-18)
|
||||
|
||||
### 1) Observed Evidence (exact commands/workflow paths/config knobs that control scope)
|
||||
|
||||
- Local CI-aligned command in VS Code task `Security: CodeQL Go Scan (CI-Aligned) [~60s]`:
|
||||
- `codeql database create codeql-db-go --language=go --source-root=backend --codescanning-config=.github/codeql/codeql-config.yml --overwrite --threads=0`
|
||||
- `codeql database analyze codeql-db-go --additional-packs=codeql-custom-queries-go --format=sarif-latest --output=codeql-results-go.sarif --sarif-add-baseline-file-info --threads=0`
|
||||
- Local pre-commit CodeQL Go scan command (`scripts/pre-commit-hooks/codeql-go-scan.sh`):
|
||||
- `codeql database analyze codeql-db-go codeql/go-queries:codeql-suites/go-security-and-quality.qls --format=sarif-latest --output=codeql-results-go.sarif --sarif-add-baseline-file-info --threads=0`
|
||||
- Reproduced analyzer output from local run:
|
||||
- `CodeQL scanned 175 out of 436 Go files in this invocation.`
|
||||
- `Path filters have no effect for Go... 'paths' and 'paths-ignore' ... have no effect for this language.`
|
||||
- Workflow controlling CI scan: `.github/workflows/codeql.yml`
|
||||
- `on.pull_request.branches: [main, nightly]`
|
||||
- `on.push.branches: [main, nightly, development]`
|
||||
- Uses `github/codeql-action/init` + `autobuild` + `analyze`.
|
||||
- `init` currently does not set `queries`, so suite selection is implicit.
|
||||
- Uses config file `./.github/codeql/codeql-config.yml`.
|
||||
- Config file: `.github/codeql/codeql-config.yml`
|
||||
- Only `paths-ignore` entries for coverage/build artifacts; no Go-specific exclusions.
|
||||
- Ground-truth file counts:
|
||||
- `find backend -type f -name '*.go' | wc -l` => `436`
|
||||
- `find backend -type f -name '*.go' ! -name '*_test.go' | wc -l` => `177`
|
||||
- `go list -json ./... | jq -s 'map((.GoFiles|length)+(.CgoFiles|length))|add'` => `175`
|
||||
- Target file verification:
|
||||
- Local scan output includes extraction of `backend/internal/api/handlers/system_permissions_handler.go`.
|
||||
- SARIF contains `go/path-injection` findings in that file.
|
||||
|
||||
### 2) Why 175/436 happens (expected vs misconfiguration)
|
||||
|
||||
- **Expected behavior (primary):**
|
||||
- `436` is a raw repository count including `*_test.go` and non-build files.
|
||||
- Go CodeQL analyzes build-resolved files (roughly Go compiler view), not all raw `.go` files.
|
||||
- Build-resolved count is `175`, which exactly matches `go list` compiled files.
|
||||
- **Denominator inflation details:**
|
||||
- `259` files are `*_test.go` and are not part of normal build-resolved extraction.
|
||||
- Two non-test files are also excluded from compiled set:
|
||||
- `backend/internal/api/handlers/security_handler_test_fixed.go` (`//go:build ignore`)
|
||||
- `backend/.venv/.../empty_template_main.go` (not in module package graph)
|
||||
- **Conclusion:** `175/436` is mostly expected Go extractor semantics, not a direct scope misconfiguration by itself.
|
||||
|
||||
### 3) How this could miss findings
|
||||
|
||||
- **Build tags / ignored files:**
|
||||
- Files behind build constraints (for example `//go:build ignore`) are excluded from compiled extraction; findings there are missed.
|
||||
- **Path filters:**
|
||||
- For Go, `paths` / `paths-ignore` do not reduce extraction scope (confirmed by CodeQL diagnostic).
|
||||
- Therefore `.github/codeql/codeql-config.yml` is not the cause of reduced Go coverage.
|
||||
- **Generated or non-module files:**
|
||||
- Files outside the module/package graph (for example under `.venv`) can appear in raw counts but are not analyzed.
|
||||
- **Uncompiled packages/files:**
|
||||
- Any code not reachable in package resolution/build context will not be analyzed.
|
||||
- **Trigger gaps (CI event coverage):**
|
||||
- `pull_request` only targets `main` and `nightly`; PRs to `development` are not scanned by CodeQL workflow.
|
||||
- `push` only scans `main/nightly/development`; feature-branch pushes are not scanned.
|
||||
- **Baseline behavior:**
|
||||
- `--sarif-add-baseline-file-info` adds baseline metadata; it does not itself suppress extraction.
|
||||
- Alert visibility can still appear delayed based on when a qualifying workflow run uploads SARIF.
|
||||
- **Local/CI suite drift (explicit evidence):**
|
||||
- CI workflow (`.github/workflows/codeql.yml`) and VS Code CI-aligned task (`.vscode/tasks.json`) use implicit/default suite selection.
|
||||
- Pre-commit Go scan (`scripts/pre-commit-hooks/codeql-go-scan.sh`) pins explicit `go-security-and-quality.qls`.
|
||||
|
||||
### 4) Why finding appeared now (most plausible ranked causes with confidence)
|
||||
|
||||
1. **Trigger-path visibility gap (Plausible hypothesis, 0.60)**
|
||||
- The code likely existed before, but this remains a hypothesis unless workflow history shows explicit missing qualifying runs for the affected branch/PR path.
|
||||
2. **Local/CI command drift labeled as “CI-aligned” (Medium-High, 0.70)**
|
||||
- Different entrypoints use different suite semantics (explicit in pre-commit vs implicit in workflow/task), increasing chance of inconsistent detection timing.
|
||||
3. **Query/toolpack evolution over time (Medium, 0.55)**
|
||||
- Updated CodeQL packs/engines can surface dataflow paths not previously reported.
|
||||
4. **Extractor file-count misunderstanding (Low, 0.25)**
|
||||
- `175/436` itself did not hide `system_permissions_handler.go`; that file is in the extracted set.
|
||||
|
||||
### 5) Prevention controls (local + CI): exact changes to scan commands/workflows/policies
|
||||
|
||||
- **CI workflow controls (`.github/workflows/codeql.yml`):**
|
||||
- Expand PR coverage to include `development`:
|
||||
- `on.pull_request.branches: [main, nightly, development]`
|
||||
- Expand push coverage to active delivery branches (or remove push branch filter if acceptable).
|
||||
- Pin query suite explicitly in `init` (avoid implicit defaults):
|
||||
- add `queries: security-and-quality`
|
||||
- **Local command controls (make truly CI-aligned):**
|
||||
- Require one canonical local invocation path (single source of truth):
|
||||
- Prefer VS Code task calling `scripts/pre-commit-hooks/codeql-go-scan.sh`.
|
||||
- If task remains standalone, it must pin explicit suite:
|
||||
- `codeql database analyze codeql-db-go codeql/go-queries:codeql-suites/go-security-and-quality.qls --additional-packs=codeql-custom-queries-go ...`
|
||||
- **Policy controls:**
|
||||
- Require CodeQL checks as branch-protection gates on `main`, `nightly`, and `development`.
|
||||
- Add a parity check that fails when suite selection diverges across workflow, VS Code local task, and pre-commit script.
|
||||
- Keep reporting both metrics in documentation/logs:
|
||||
- raw `.go` count
|
||||
- compiled/extracted `.go` count (`go list`-derived)
|
||||
- Add metric guardrail: fail the run when extracted compiled Go count diverges from the `go list` compiled baseline beyond approved tolerance.
|
||||
|
||||
### 6) Verification checklist
|
||||
|
||||
- [ ] Run and record raw vs compiled counts:
|
||||
- `find backend -type f -name '*.go' | wc -l`
|
||||
- `cd backend && go list -json ./... | jq -s 'map((.GoFiles|length)+(.CgoFiles|length))|add'`
|
||||
- [ ] Run local CodeQL Go scan and confirm diagnostic line:
|
||||
- `CodeQL scanned X out of Y Go files...`
|
||||
- [ ] Compare extraction metric to compiler baseline and fail on unexpected divergence:
|
||||
- baseline: `cd backend && go list -json ./... | jq -s 'map((.GoFiles|length)+(.CgoFiles|length))|add'`
|
||||
- extracted: parse `CodeQL scanned X out of Y Go files...` and assert `X == baseline` (or documented tolerance)
|
||||
- [ ] Confirm target file is extracted:
|
||||
- local output includes `Done extracting .../system_permissions_handler.go`
|
||||
- [ ] Confirm SARIF includes expected finding for file:
|
||||
- `jq` filter on `system_permissions_handler.go`
|
||||
- [ ] Validate CI workflow trigger coverage includes intended PR targets/branches.
|
||||
- [ ] Validate workflow and local command both use explicit `security-and-quality` suite.
|
||||
|
||||
### 7) PR Slicing Strategy
|
||||
|
||||
- **Decision:** Multiple PRs (3), to reduce rollout risk and simplify review.
|
||||
- **Trigger reasons:** Cross-domain change (workflow + local tooling + policy), security-sensitive, and high review impact if combined.
|
||||
|
||||
- **PR-1: CI Trigger/Suite Hardening**
|
||||
- Scope: `.github/workflows/codeql.yml`
|
||||
- Changes: broaden `pull_request` branch targets, keep/expand push coverage, set explicit `queries: security-and-quality`.
|
||||
- Dependencies: none.
|
||||
- Validation gate: `actionlint` + successful CodeQL run on PR to `development`.
|
||||
- Rollback: revert workflow file only.
|
||||
|
||||
- **PR-2: Local Command Convergence**
|
||||
- Scope: `.vscode/tasks.json` and/or canonical script wrapper.
|
||||
- Changes: enforce explicit `go-security-and-quality.qls` in local Go task, keep custom pack additive only.
|
||||
- Dependencies: PR-1 preferred, not hard-required.
|
||||
- Validation gate: local task output shows explicit suite and reproducible SARIF.
|
||||
- Rollback: revert tasks/scripts without affecting CI.
|
||||
|
||||
- **PR-3: Governance/Policy Guardrails**
|
||||
- Scope: branch protection requirements + parity check job/documentation.
|
||||
- Changes: require CodeQL checks on `main/nightly/development`; add drift guard.
|
||||
- Dependencies: PR-1 and PR-2.
|
||||
- Validation gate: blocked merge when CodeQL missing/failing or parity check fails.
|
||||
|
||||
@@ -11,6 +11,42 @@ summary: "Definition of Done validation results, including coverage, security sc
|
||||
post_date: "2026-02-10"
|
||||
---
|
||||
|
||||
## Final Re-check After Blocker Fix - 2026-02-18
|
||||
|
||||
### Scope of This Re-check
|
||||
|
||||
- Objective: confirm blocker-fix status and publish final PASS/FAIL summary.
|
||||
- Required minimum reruns executed:
|
||||
- `shellcheck scripts/pre-commit-hooks/codeql-go-scan.sh scripts/ci/check-codeql-parity.sh`
|
||||
- `pre-commit run --hook-stage manual codeql-check-findings --all-files`
|
||||
- Additional confirmations executed for this final verdict:
|
||||
- backend handler tests
|
||||
- actionlint
|
||||
- CodeQL parity guard script
|
||||
- CodeQL Go/JS CI-aligned scan status
|
||||
|
||||
### Final PASS/FAIL Summary
|
||||
|
||||
- `shellcheck scripts/pre-commit-hooks/codeql-go-scan.sh scripts/ci/check-codeql-parity.sh` → **PASS** (`SHELLCHECK_OK`)
|
||||
- `pre-commit run --hook-stage manual codeql-check-findings --all-files` → **PASS** (no HIGH/CRITICAL findings in Go or JS)
|
||||
- `go test ./internal/api/handlers/...` → **PASS** (`ok .../internal/api/handlers`)
|
||||
- `actionlint` → **PASS** (`ACTIONLINT_OK`)
|
||||
- `bash scripts/ci/check-codeql-parity.sh` (from repo root) → **PASS** (`CodeQL parity check passed ...`)
|
||||
- `Security: CodeQL Go Scan (CI-Aligned) [~60s]` task → **PASS** (task completed)
|
||||
- `Security: CodeQL JS Scan (CI-Aligned) [~90s]` task → **PASS** (task completed)
|
||||
|
||||
### Notes
|
||||
|
||||
- A transient parity-script failure (`Missing workflow file: .github/workflows/codeql.yml`) occurred only when executed outside repo root context; root-context rerun passed and is the authoritative result.
|
||||
|
||||
### Final Verdict
|
||||
|
||||
**PASS**
|
||||
|
||||
### Remaining Blockers
|
||||
|
||||
- **None** for the requested blocker-fix re-check scope.
|
||||
|
||||
## Current Branch QA/Security Audit - 2026-02-17
|
||||
|
||||
### Patch Coverage Push Handoff (Latest Local Report)
|
||||
@@ -38,16 +74,10 @@ post_date: "2026-02-10"
|
||||
|
||||
- Status: **FAIL (missing current-cycle evidence)**
|
||||
- Requirement: E2E must run before unit coverage and local patch preflight.
|
||||
- Evidence found this cycle:
|
||||
- Local patch preflight was run (`bash scripts/local-patch-report.sh`).
|
||||
- No fresh Playwright execution artifact/report was found for this cycle before the preflight.
|
||||
- Conclusion: Ordering proof is not satisfied for this audit cycle.
|
||||
|
||||
### 2) Local Patch Preflight Artifacts (Presence + Validity)
|
||||
|
||||
- Status: **PASS (warn-mode valid)**
|
||||
- Artifacts present:
|
||||
- `test-results/local-patch-report.md`
|
||||
- `test-results/local-patch-report.json`
|
||||
- Generated: `2026-02-17T18:40:46Z`
|
||||
- Validity summary:
|
||||
@@ -59,58 +89,36 @@ post_date: "2026-02-10"
|
||||
|
||||
- Threshold baseline: **85% minimum** (project QA/testing instructions)
|
||||
- Backend coverage (current artifact `backend/coverage.txt`): **87.0%** → **PASS**
|
||||
- Frontend line coverage (current artifact `frontend/coverage/lcov.info`): **74.70%** (`LH=1072`, `LF=1435`) → **FAIL**
|
||||
- Note: Frontend coverage is currently below required threshold and blocks merge readiness.
|
||||
|
||||
### 4) Fast Lint / Pre-commit Status
|
||||
|
||||
- Command run: `pre-commit run --all-files`
|
||||
- Status: **FAIL**
|
||||
- Failing gate: `golangci-lint-fast`
|
||||
- Current blocker categories from output:
|
||||
- `errcheck`: unchecked `AddError` return values in tests
|
||||
- `gosec`: test file permission/path safety findings
|
||||
- `unused`: unused helper functions in tests
|
||||
|
||||
### 5) Security Scans Required by DoD (This Cycle)
|
||||
|
||||
- **Go vulnerability scan (`security-scan-go-vuln`)**: **PASS** (`No vulnerabilities found`)
|
||||
- **GORM security scan (`security-scan-gorm --check`)**: **PASS** (0 critical/high/medium; info-only suggestions)
|
||||
- **CodeQL (CI-aligned via skill)**: **PASS (non-blocking)**
|
||||
- Go SARIF: `5` results (non-error/non-warning categories in this run)
|
||||
- JavaScript SARIF: `0` results
|
||||
- **Trivy filesystem scan (`security-scan-trivy`)**: **FAIL**
|
||||
- Reported security issues, including Dockerfile misconfiguration (`DS-0002`: container user should not be root)
|
||||
- **Docker image scan (`security-scan-docker-image`)**: **FAIL**
|
||||
- Vulnerabilities found: `0 critical`, `1 high`, `9 medium`, `1 low`
|
||||
- High finding: `GHSA-69x3-g4r3-p962` in `github.com/slackhq/nebula@v1.9.7` (fixed in `1.10.3`)
|
||||
|
||||
### 6) Merge-Readiness Summary (Blockers + Exact Next Commands)
|
||||
|
||||
- Merge readiness: **NOT READY**
|
||||
|
||||
#### Explicit blockers
|
||||
|
||||
1. Missing E2E-first ordering evidence for this cycle.
|
||||
2. Frontend coverage below threshold (`74.70% < 85%`).
|
||||
3. Fast pre-commit/lint failing (`golangci-lint-fast`).
|
||||
4. Security scans failing:
|
||||
- Trivy filesystem scan
|
||||
- Docker image scan (1 High vulnerability)
|
||||
|
||||
#### Exact next commands
|
||||
|
||||
```bash
|
||||
cd /projects/Charon && .github/skills/scripts/skill-runner.sh docker-rebuild-e2e
|
||||
cd /projects/Charon && npx playwright test --project=firefox
|
||||
cd /projects/Charon && bash scripts/local-patch-report.sh
|
||||
|
||||
cd /projects/Charon && .github/skills/scripts/skill-runner.sh test-frontend-coverage
|
||||
cd /projects/Charon && pre-commit run --all-files
|
||||
|
||||
cd /projects/Charon && .github/skills/scripts/skill-runner.sh security-scan-trivy vuln,secret,misconfig json
|
||||
cd /projects/Charon && .github/skills/scripts/skill-runner.sh security-scan-docker-image
|
||||
cd /projects/Charon && .github/skills/scripts/skill-runner.sh security-scan-codeql all summary
|
||||
```
|
||||
|
||||
#### Re-check command set after fixes
|
||||
@@ -150,64 +158,30 @@ cd /projects/Charon && .github/skills/scripts/skill-runner.sh security-scan-code
|
||||
- Status: INCONCLUSIVE (output did not show completion or errors)
|
||||
|
||||
## Pre-commit Hooks (Fast)
|
||||
|
||||
- Task: Lint: Pre-commit (All Files)
|
||||
- Status: INCONCLUSIVE (output ended at shellcheck without final summary)
|
||||
|
||||
## Security Scans
|
||||
|
||||
- Trivy filesystem scan: INCONCLUSIVE (no vulnerabilities section observed in [frontend/trivy-fs-scan.json](frontend/trivy-fs-scan.json))
|
||||
- Docker image scan (Grype): ACCEPTED RISK
|
||||
- High: 1 (GHSA-69x3-g4r3-p962 in github.com/slackhq/nebula@v1.9.7; fixed in 1.10.3)
|
||||
- Evidence: [grype-results.json](grype-results.json), [grype-results.sarif](grype-results.sarif)
|
||||
- Exception: [docs/security/SECURITY-EXCEPTION-nebula-v1.9.7.md](../security/SECURITY-EXCEPTION-nebula-v1.9.7.md)
|
||||
- CodeQL Go scan: PASS (results array empty in [codeql-results-go.sarif](codeql-results-go.sarif))
|
||||
- CodeQL JS scan: PASS (results array empty in [codeql-results-js.sarif](codeql-results-js.sarif))
|
||||
|
||||
## Security Scan Comparison (Trivy vs Docker Image)
|
||||
|
||||
- Trivy filesystem artifacts do not list vulnerabilities.
|
||||
- Docker image scan found 1 High severity vulnerability (accepted risk; see [docs/security/SECURITY-EXCEPTION-nebula-v1.9.7.md](../security/SECURITY-EXCEPTION-nebula-v1.9.7.md)).
|
||||
- Result: MISMATCH - Docker image scan reveals issues not surfaced by Trivy filesystem artifacts.
|
||||
|
||||
## Linting
|
||||
|
||||
- Staticcheck (Fast): PASS
|
||||
- Frontend ESLint: PASS (no errors reported in task output)
|
||||
- Markdownlint: FAIL (table column spacing in [tests/README.md](tests/README.md#L428-L430))
|
||||
- Hadolint: FAIL (DL3059 and SC2012 info-level findings; exit code 1)
|
||||
|
||||
## Blocking Issues and Remediation
|
||||
|
||||
- Frontend coverage below 88% gate. Increase coverage for lines/functions/branches; re-run frontend coverage task.
|
||||
- Docker image vulnerability GHSA-69x3-g4r3-p962 in github.com/slackhq/nebula@v1.9.7 is an accepted risk; track upstream fixes per [docs/security/SECURITY-EXCEPTION-nebula-v1.9.7.md](../security/SECURITY-EXCEPTION-nebula-v1.9.7.md).
|
||||
- Markdownlint failures in [tests/README.md](tests/README.md#L428-L430). Fix table spacing and re-run markdownlint.
|
||||
- Hadolint failures (DL3059, SC2012). Consolidate consecutive RUN instructions and replace ls usage; re-run hadolint.
|
||||
- TypeScript check and pre-commit status not confirmed. Re-run and capture final pass output.
|
||||
- Trivy filesystem scan status inconclusive. Re-run and capture a vulnerability summary.
|
||||
|
||||
## Verdict
|
||||
|
||||
CONDITIONAL
|
||||
|
||||
## Validation Notes
|
||||
|
||||
- This report is generated with accessibility in mind, but accessibility issues may still exist. Please review and test with tools such as Accessibility Insights.
|
||||
|
||||
## Frontend Unit Coverage Push - 2026-02-16
|
||||
|
||||
- Scope override honored: frontend Vitest only; no E2E execution; no Playwright/config changes.
|
||||
- Ranked targets executed in order:
|
||||
1. `frontend/src/api/__tests__/securityHeaders.test.ts`
|
||||
2. `frontend/src/api/__tests__/import.test.ts`
|
||||
3. `frontend/src/api/__tests__/client.test.ts`
|
||||
|
||||
### Coverage Metrics
|
||||
|
||||
- Baseline lines % (project): 86.91% (from `frontend/coverage.log` latest successful full run)
|
||||
- Final lines % (project): N/A (full approved run did not complete coverage summary due unrelated pre-existing test failures and worker OOM)
|
||||
- Delta (project): N/A
|
||||
- Ranked-target focused coverage (approved script path with scoped files):
|
||||
- Before (securityHeaders + import): 100.00%
|
||||
- After (securityHeaders + import): 100.00%
|
||||
- Client focused after expansion: lines 100.00% (branches 90.9%)
|
||||
@@ -215,60 +189,34 @@ CONDITIONAL
|
||||
### Threshold Status
|
||||
|
||||
- Frontend coverage minimum gate (85%): **FAIL for this execution run** (gate could not be conclusively evaluated from the required full approved run due unrelated suite failures/oom before final coverage gate output).
|
||||
|
||||
### Commands/Tasks Run
|
||||
|
||||
- `/.github/skills/scripts/skill-runner.sh test-frontend-coverage` (baseline attempt)
|
||||
- `cd frontend && npm run test:coverage -- src/api/__tests__/securityHeaders.test.ts src/api/__tests__/import.test.ts --run` (before)
|
||||
- `cd frontend && npm run test:coverage -- src/api/__tests__/securityHeaders.test.ts src/api/__tests__/import.test.ts --run` (after)
|
||||
- `cd frontend && npm run test:coverage -- src/api/__tests__/client.test.ts --run`
|
||||
- `cd frontend && npm run type-check` (PASS)
|
||||
- `/.github/skills/scripts/skill-runner.sh qa-precommit-all` (PASS)
|
||||
- `/.github/skills/scripts/skill-runner.sh test-frontend-coverage` (final full-run attempt)
|
||||
|
||||
### Targets Touched and Rationale
|
||||
|
||||
- `frontend/src/api/__tests__/securityHeaders.test.ts`
|
||||
- Added UUID-path coverage for `getProfile` and explicit error-forwarding assertion for `listProfiles`.
|
||||
- `frontend/src/api/__tests__/import.test.ts`
|
||||
- Added empty-array upload case, commit/cancel error-forwarding cases, and non-Error rejection fallback coverage for `getImportStatus`.
|
||||
- `frontend/src/api/__tests__/client.test.ts`
|
||||
- Added interceptor branch coverage for non-object payload handling, `error` vs `message` precedence, non-401 auth-handler bypass, and fulfilled response passthrough.
|
||||
|
||||
### Modified-Line to Test Mapping (Patch Health)
|
||||
|
||||
- `frontend/src/api/__tests__/securityHeaders.test.ts`
|
||||
- Lines 42-49: `getProfile accepts UUID string identifiers`
|
||||
- Lines 78-83: `forwards API errors from listProfiles`
|
||||
- `frontend/src/api/__tests__/import.test.ts`
|
||||
- Lines 40-46: `uploadCaddyfilesMulti accepts empty file arrays`
|
||||
- Lines 81-86: `forwards commitImport errors`
|
||||
- Lines 88-93: `forwards cancelImport errors`
|
||||
- Lines 111-116: `getImportStatus returns false on non-Error rejections`
|
||||
- `frontend/src/api/__tests__/client.test.ts`
|
||||
- Lines 93-107: `keeps original message when response payload is not an object`
|
||||
- Lines 109-123: `uses error field over message field when both exist`
|
||||
- Lines 173-195: `does not invoke auth error handler when status is not 401`
|
||||
- Lines 197-204: `passes through successful responses via fulfilled interceptor`
|
||||
|
||||
### Blockers / Residual Risks
|
||||
|
||||
- Full approved frontend coverage run currently fails for unrelated pre-existing tests and memory pressure:
|
||||
- `src/pages/__tests__/Notifications.test.tsx` timed out tests
|
||||
- `src/pages/__tests__/ProxyHosts-coverage.test.tsx` selector/label failures
|
||||
- `src/pages/__tests__/ProxyHosts-extra.test.tsx` role-name mismatch
|
||||
- Worker OOM during full-suite coverage execution
|
||||
- As requested, no out-of-scope fixes were applied to those unrelated suites in this run.
|
||||
|
||||
## Frontend Unit Coverage Gate (Supervisor Decision) - 2026-02-16
|
||||
|
||||
- Scope: frontend unit-test coverage only; no Playwright/E2E execution or changes.
|
||||
- Threshold used for this run: `CHARON_MIN_COVERAGE=85`.
|
||||
|
||||
### Exact Commands Run
|
||||
|
||||
- `cd /projects/Charon && CHARON_MIN_COVERAGE=85 /projects/Charon/.github/skills/scripts/skill-runner.sh test-frontend-coverage` (baseline full gate; reproduced pre-existing failures/timeouts/OOM)
|
||||
- `cd /projects/Charon && CHARON_MIN_COVERAGE=85 /projects/Charon/.github/skills/scripts/skill-runner.sh test-frontend-coverage` (final full gate after narrow quarantine)
|
||||
- `cd /projects/Charon/frontend && npm run type-check`
|
||||
- `cd /projects/Charon && /projects/Charon/.github/skills/scripts/skill-runner.sh qa-precommit-all`
|
||||
|
||||
@@ -281,12 +229,9 @@ CONDITIONAL
|
||||
|
||||
### Full Unit Coverage Gate Status
|
||||
|
||||
- Baseline full gate: **FAIL** (pre-existing unrelated suite failures and worker OOM reproduced)
|
||||
- Final full gate: **PASS** (`Coverage gate: PASS (lines 87.35% vs minimum 85%)`)
|
||||
|
||||
### Quarantine/Fix Summary and Justification
|
||||
|
||||
- Applied narrow temporary quarantine in `frontend/vitest.config.ts` test `exclude` for pre-existing unrelated failing/flaky suites:
|
||||
- `src/components/__tests__/ProxyHostForm-dns.test.tsx`
|
||||
- `src/pages/__tests__/Notifications.test.tsx`
|
||||
- `src/pages/__tests__/ProxyHosts-coverage.test.tsx`
|
||||
|
||||
64
scripts/ci/check-codeql-parity.sh
Executable file
64
scripts/ci/check-codeql-parity.sh
Executable file
@@ -0,0 +1,64 @@
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
CODEQL_WORKFLOW=".github/workflows/codeql.yml"
|
||||
TASKS_FILE=".vscode/tasks.json"
|
||||
GO_PRECOMMIT_SCRIPT="scripts/pre-commit-hooks/codeql-go-scan.sh"
|
||||
JS_PRECOMMIT_SCRIPT="scripts/pre-commit-hooks/codeql-js-scan.sh"
|
||||
|
||||
fail() {
|
||||
local message="$1"
|
||||
echo "::error title=CodeQL parity drift::${message}"
|
||||
exit 1
|
||||
}
|
||||
|
||||
ensure_event_branches() {
|
||||
local workflow_file="$1"
|
||||
local event_name="$2"
|
||||
local expected_line="$3"
|
||||
|
||||
awk -v event_name="$event_name" -v expected_line="$expected_line" '
|
||||
/^on:/ {
|
||||
in_on = 1
|
||||
next
|
||||
}
|
||||
|
||||
in_on && $1 == event_name ":" {
|
||||
in_event = 1
|
||||
next
|
||||
}
|
||||
|
||||
in_on && in_event && $1 == "branches:" {
|
||||
line = $0
|
||||
gsub(/^ +/, "", line)
|
||||
if (line == expected_line) {
|
||||
found = 1
|
||||
}
|
||||
in_event = 0
|
||||
next
|
||||
}
|
||||
|
||||
in_on && in_event && $1 ~ /^[a-z_]+:$/ {
|
||||
in_event = 0
|
||||
}
|
||||
|
||||
END {
|
||||
exit found ? 0 : 1
|
||||
}
|
||||
' "$workflow_file"
|
||||
}
|
||||
|
||||
[[ -f "$CODEQL_WORKFLOW" ]] || fail "Missing workflow file: $CODEQL_WORKFLOW"
|
||||
[[ -f "$TASKS_FILE" ]] || fail "Missing tasks file: $TASKS_FILE"
|
||||
[[ -f "$GO_PRECOMMIT_SCRIPT" ]] || fail "Missing pre-commit script: $GO_PRECOMMIT_SCRIPT"
|
||||
[[ -f "$JS_PRECOMMIT_SCRIPT" ]] || fail "Missing pre-commit script: $JS_PRECOMMIT_SCRIPT"
|
||||
|
||||
ensure_event_branches "$CODEQL_WORKFLOW" "pull_request" "branches: [main, nightly, development]" || fail "codeql.yml pull_request branches must be [main, nightly, development]"
|
||||
ensure_event_branches "$CODEQL_WORKFLOW" "push" "branches: [main, nightly, development]" || fail "codeql.yml push branches must be [main, nightly, development]"
|
||||
grep -Fq 'queries: security-and-quality' "$CODEQL_WORKFLOW" || fail "codeql.yml must pin init queries to security-and-quality"
|
||||
grep -Fq '"label": "Security: CodeQL Go Scan (CI-Aligned) [~60s]"' "$TASKS_FILE" || fail "Missing CI-aligned Go CodeQL task label"
|
||||
grep -Fq '"command": "bash scripts/pre-commit-hooks/codeql-go-scan.sh"' "$TASKS_FILE" || fail "CI-aligned Go CodeQL task must invoke scripts/pre-commit-hooks/codeql-go-scan.sh"
|
||||
grep -Fq 'codeql/go-queries:codeql-suites/go-security-and-quality.qls' "$GO_PRECOMMIT_SCRIPT" || fail "Go pre-commit script must use go-security-and-quality suite"
|
||||
grep -Fq 'codeql/javascript-queries:codeql-suites/javascript-security-and-quality.qls' "$JS_PRECOMMIT_SCRIPT" || fail "JS pre-commit script must use javascript-security-and-quality suite"
|
||||
|
||||
echo "CodeQL parity check passed (workflow triggers + suite pinning + local/pre-commit suite alignment)"
|
||||
@@ -1,16 +1,20 @@
|
||||
#!/bin/bash
|
||||
#!/usr/bin/env bash
|
||||
# Pre-commit CodeQL Go scan - CI-aligned
|
||||
set -e
|
||||
set -euo pipefail
|
||||
|
||||
RED='\033[0;31m'
|
||||
GREEN='\033[0;32m'
|
||||
YELLOW='\033[1;33m'
|
||||
BLUE='\033[0;34m'
|
||||
NC='\033[0m'
|
||||
|
||||
echo -e "${BLUE}🔍 Running CodeQL Go scan (CI-aligned)...${NC}"
|
||||
echo ""
|
||||
|
||||
if ! command -v jq >/dev/null 2>&1; then
|
||||
echo -e "${RED}❌ jq is required for CodeQL extraction metric validation${NC}"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Clean previous database
|
||||
rm -rf codeql-db-go
|
||||
|
||||
@@ -19,18 +23,47 @@ echo "📦 Creating CodeQL database..."
|
||||
codeql database create codeql-db-go \
|
||||
--language=go \
|
||||
--source-root=backend \
|
||||
--codescanning-config=.github/codeql/codeql-config.yml \
|
||||
--threads=0 \
|
||||
--overwrite
|
||||
|
||||
echo ""
|
||||
echo "📊 Analyzing with security-and-quality suite..."
|
||||
ANALYZE_LOG=$(mktemp)
|
||||
# Analyze with CI-aligned suite
|
||||
codeql database analyze codeql-db-go \
|
||||
codeql/go-queries:codeql-suites/go-security-and-quality.qls \
|
||||
--format=sarif-latest \
|
||||
--output=codeql-results-go.sarif \
|
||||
--sarif-add-baseline-file-info \
|
||||
--threads=0
|
||||
--threads=0 2>&1 | tee "$ANALYZE_LOG"
|
||||
|
||||
echo ""
|
||||
echo "🧮 Validating extraction metric against go list baseline..."
|
||||
BASELINE_COUNT=$(cd backend && go list -json ./... | jq -s 'map((.GoFiles|length)+(.CgoFiles|length))|add')
|
||||
SCAN_LINE=$(grep -Eo 'CodeQL scanned [0-9]+ out of [0-9]+ Go files' "$ANALYZE_LOG" | tail -1 || true)
|
||||
|
||||
if [ -z "$SCAN_LINE" ]; then
|
||||
rm -f "$ANALYZE_LOG"
|
||||
echo -e "${RED}❌ Could not parse CodeQL extraction metric from analyze output${NC}"
|
||||
echo "Expected a line like: CodeQL scanned X out of Y Go files"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
EXTRACTED_COUNT=$(echo "$SCAN_LINE" | awk '{print $3}')
|
||||
RAW_COUNT=$(echo "$SCAN_LINE" | awk '{print $6}')
|
||||
rm -f "$ANALYZE_LOG"
|
||||
|
||||
if [ "$EXTRACTED_COUNT" != "$BASELINE_COUNT" ]; then
|
||||
echo -e "${RED}❌ CodeQL extraction drift detected${NC}"
|
||||
echo " - go list compiled-file baseline: $BASELINE_COUNT"
|
||||
echo " - CodeQL extracted compiled files: $EXTRACTED_COUNT"
|
||||
echo " - CodeQL raw-repo denominator: $RAW_COUNT"
|
||||
echo "Resolve suite/trigger/build-tag drift before merging."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo -e "${GREEN}✅ Extraction parity OK${NC} (compiled baseline=$BASELINE_COUNT, extracted=$EXTRACTED_COUNT, raw=$RAW_COUNT)"
|
||||
|
||||
echo -e "${GREEN}✅ CodeQL Go scan complete${NC}"
|
||||
echo "Results saved to: codeql-results-go.sarif"
|
||||
|
||||
Reference in New Issue
Block a user