Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/security/CODEQL_ALERT_LOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Following Eric Elliott's zero-trust philosophy: **Never trust user input. Assume
**Branch**: `codeql-hardening`
**Total Alerts**: 77
**Resolved**: 76/77 (1 dismissed as S4 false positive)
**Additional Dismissed**: 13 (7 Google OAuth S4 + 2 Round 4 false positives + 4 Round 5 S3)

---

Expand Down Expand Up @@ -136,6 +137,17 @@ Following Eric Elliott's zero-trust philosophy: **Never trust user input. Assume
| 138 | js/http-to-file-access | medium | page-content-store.ts:161 | CWE-073 | Content-addressable storage: file path is SHA-256 hash validated by `/^[a-f0-9]{64}$/i` (path traversal impossible). All callers require authentication + `canUserEditPage()` authorization. Atomic `wx` flag write. CodeQL can't model hash derivation breaking taint chain | DISMISSED (false positive) |
| 139 | js/request-forgery | critical | fetch-proxy-handler.ts:49 | CWE-918 | URL validated by `isAllowedFetchProxyURL()` before fetch — strict allowlist (localhost/private IPs, http/https only). Redirects blocked (lines 57-61). Not exposed to renderer; requires authenticated WebSocket. 102+ test cases cover validation. CodeQL can't model allowlist breaking taint chain | DISMISSED (false positive) |

### Round 5: Incomplete Sanitization in Changelog Scripts (132-135)

| Alert | Rule | Severity | File | CWE | Triage Rationale | Status |
|-------|------|----------|------|-----|-----------------|--------|
| 132 | js/incomplete-sanitization | high | detect-abandoned-approaches.ts:192 | CWE-116 | `.replace(/\|/g, "\\|")` escapes pipes but not backslashes. Output is markdown table in `docs/changelog/evidence/` — display only, no code execution path. Input is git commit subjects (semi-trusted). S3 per rubric: display/logging output | DISMISSED (S3) |
| 133 | js/incomplete-sanitization | high | detect-abandoned-approaches.ts:193 | CWE-116 | Same pattern as #132 on adjacent line (`file.deleted.message`). Output is markdown documentation, not RegExp or shell exec | DISMISSED (S3) |
| 134 | js/incomplete-sanitization | high | detect-multiple-attempts.ts:219 | CWE-116 | `.replace(/\|/g, "\\|")` for markdown table cell. Output goes to `docs/changelog/evidence/patterns/multiple-attempts.md`. Developer tooling, not production code | DISMISSED (S3) |
| 135 | js/incomplete-sanitization | high | track-file-evolution.ts:174 | CWE-116 | `.replace(/\|/g, "\\|")` for markdown table cell. Output goes to `docs/changelog/evidence/files/`. No downstream parsing or execution of the output | DISMISSED (S3) |

**Precedent**: Alert #28 (`prettier.ts:57`) was rated S2 and FIXED because its incompletely-sanitized string fed into `new RegExp()`. These 4 alerts output exclusively to static markdown documentation files — fundamentally different risk profile.

---

### Files Modified (26 files)
Expand Down
33 changes: 33 additions & 0 deletions tasks/security-triage-batch-7.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Security Alert Triage — Batch 7: Incomplete Sanitization in Changelog Scripts

## Summary

4 CodeQL `js/incomplete-sanitization` (CWE-116) HIGH alerts in changelog analysis scripts. All 4 flag the same pattern: `.replace(/\|/g, "\\|")` escapes pipe characters for markdown table cells but does not escape backslashes. All rated **S3 — Dismiss** because the output is markdown documentation files (display only), not RegExp, shell exec, or SQL.

## Alert #132 — js/incomplete-sanitization
- **File**: `scripts/changelog/detect-abandoned-approaches.ts:192`
- **Rating**: S3 — False Positive / Low Priority
- **Rationale**: `file.created.message` from `git log --diff-filter=A --format="COMMIT:%H|%ad|%s"` is escaped with `.replace(/\|/g, "\\|")` and inserted into a markdown table. Output written to `docs/changelog/evidence/patterns/abandoned-approaches.md`. No code execution path from this output — it is static documentation. Input is semi-trusted (git commit subjects, requires commit access to the repository).
- **Action**: Dismiss — "S3: Developer tooling script. Output is markdown documentation only. No code execution path. Input is git commit subjects."

## Alert #133 — js/incomplete-sanitization
- **File**: `scripts/changelog/detect-abandoned-approaches.ts:193`
- **Rating**: S3 — False Positive / Low Priority
- **Rationale**: Same pattern as #132 on adjacent line, using `file.deleted.message` from `git log --diff-filter=D`. Same output destination, same risk profile.
- **Action**: Dismiss — same rationale as #132.

## Alert #134 — js/incomplete-sanitization
- **File**: `scripts/changelog/detect-multiple-attempts.ts:219`
- **Rating**: S3 — False Positive / Low Priority
- **Rationale**: `commit.subject` from `git log --format="COMMIT:%H|%ad|%s|BODY_START%b|BODY_END"` is escaped with `.slice(0, 60).replace(/\|/g, "\\|")` and inserted into a markdown table. Output written to `docs/changelog/evidence/patterns/multiple-attempts.md`. Developer tooling only — not referenced in CI or production builds.
- **Action**: Dismiss — "S3: Developer tooling script. Output is markdown documentation only. No code execution path."

## Alert #135 — js/incomplete-sanitization
- **File**: `scripts/changelog/track-file-evolution.ts:174`
- **Rating**: S3 — False Positive / Low Priority
- **Rationale**: `commit.message` from `git log --follow --format="COMMIT:%H|%ad|%s"` is escaped with `.replace(/\|/g, "\\|").slice(0, 60)` and inserted into a markdown table. Output written to `docs/changelog/evidence/files/<filename>.md`. Static documentation output with no downstream parsing or execution.
- **Action**: Dismiss — "S3: Developer tooling script. Output is markdown documentation only. No code execution path."

## Precedent

Alert #28 (`prettier.ts:57`) was the same CWE-116 incomplete-sanitization rule but was rated S2 and FIXED because the incompletely-escaped string was passed to `new RegExp()`, where backslashes alter regex semantics. These 4 alerts output exclusively to static `.md` files — no interpreter processes the output.
Loading