Skip to content

Commit 60056dc

Browse files
committed
test: add GitHubClientOriginTests and enhance CleanupPathGuard security
- Introduced comprehensive tests for `GitHubClient` to validate API origin enforcement and token security. - Expanded `CleanupPathGuard` functionality with safe quarantine handling, isolated directory decisions, and secure mutation verification. - Added several helper methods to improve directory traversal reliability and secure file operations.
1 parent 9d4389d commit 60056dc

10 files changed

Lines changed: 2812 additions & 297 deletions

.github/CODEQL-WORKFLOW.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,14 @@ local files, command-line arguments, environment variables, and other local
2323
sources used by maintenance scripts. Neither configuration excludes C#
2424
findings by rule.
2525

26+
This split is deliberate. The compiled web application has a remote request
27+
boundary, and treating all application-owned database and filesystem state as
28+
attacker-controlled creates unactionable C# flows. Python maintenance tools
29+
really do accept local files, environment variables, and command-line values
30+
from an administrator, so the local threat model accurately represents their
31+
trust boundary. Do not enable `threat-models: local` globally to make the two
32+
very different execution models look uniform.
33+
2634
The local models are intentionally narrow. They identify
2735
`LogSanitizer.Sanitize` as a barrier only for the `log-injection` flow kind and
2836
`ConfigurationLogRedactor.RedactValue` as a barrier only when values would be
@@ -82,8 +90,30 @@ This administrative cleanup is required for findings that exist only in the
8290
former default-setup configuration; a repository commit cannot delete that
8391
server-side configuration.
8492

93+
The stale Python configurations must not be removed before the checked-in
94+
Python matrix job succeeds on `main`. Otherwise, the repository can temporarily
95+
lose active Python coverage rather than transferring it to the advanced
96+
workflow.
97+
8598
## Verification
8699

100+
The July 2026 local verification reports:
101+
102+
- the six original C# alerts have source fixes and regression coverage, while
103+
the Python setup alert has a hardened necessary sink and narrow documented
104+
suppression;
105+
- a fresh default-remote C# database moved from eight findings to zero;
106+
- fresh GitHub Actions and JavaScript/TypeScript databases report zero
107+
findings;
108+
- a fresh workflow-equivalent Python database ran all 45 default queries with
109+
the local threat model and reported zero findings and no SARIF warning/error
110+
notifications; and
111+
- the 52-query Python security-extended suite also reported zero findings.
112+
113+
The Python filesystem audit passes 57 focused tests, and all 109 script tests
114+
pass with `ResourceWarning` promoted to an error. These complete-suite results,
115+
not an earlier targeted-query result, form the local release gate.
116+
87117
After changing the workflow or CodeQL configuration:
88118

89119
1. Validate `.github/workflows/codeql.yml` with `actionlint`.

design/docs/20260711-code-scanning-remediation-changes.md

Lines changed: 114 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,15 @@
77

88
## Summary
99

10-
This work reduces GitHub CodeQL and Trivy findings through source-level privacy
11-
fixes, setup-secret hardening, and container dependency remediation.
10+
This work addresses the 423-alert baseline (416 Trivy and seven CodeQL) through
11+
source-level privacy fixes, setup-secret and filesystem hardening, workflow
12+
supply-chain controls, and production-container dependency remediation. The
13+
verified image inventory is 140 non-fixable medium/low findings. Six original
14+
C# alerts and the fresh C# findings have source fixes; the Python setup alert is
15+
handled by a hardened, necessary persistence boundary and narrow documented
16+
suppression. Fresh Actions and JavaScript/TypeScript scans report zero
17+
findings. Fresh workflow-equivalent default and security-extended Python scans
18+
also report zero findings under the local threat model.
1219

1320
## Changes
1421

@@ -47,11 +54,19 @@ fixes, setup-secret hardening, and container dependency remediation.
4754
explicit overwrite, POSIX permissions, symlink refusal, and non-regular
4855
destination handling for the interactive container setup path.
4956
- `scripts/tests/test_code_scanning_security.py` - Covers bounded Link-header
50-
parsing under adversarial input and secret-free demo-user success/failure
51-
output.
57+
parsing under adversarial input, exact-origin HTTPS request and redirect
58+
enforcement, bounded redirect behavior, and secret-free demo-user
59+
success/failure output.
5260
- `scripts/tests/test_incoming_clean_up.py` - Covers canonical root boundaries,
5361
symlink and outside-root refusal, Zip Slip/symlink-member rejection, SFV
54-
traversal, safe encoding repair, pretend mode, and contained live deletion.
62+
traversal, private extraction permissions, descriptor-relative race defense,
63+
safe encoding repair, pretend mode, and contained live deletion.
64+
- `tests/Melodee.Tests.Common/Services/SecurityLogSanitizationTests.cs` - Covers
65+
the untrusted player, playlist, script setting, configuration key, provider,
66+
and artist values reported by the fresh C# CodeQL scan.
67+
- `tests/Melodee.Tests.Common/TestHelpers/RecordingLogEventSink.cs` - Captures
68+
rendered security-sensitive log events for assertions without external
69+
logging infrastructure.
5570

5671
### Modified
5772

@@ -83,14 +98,18 @@ fixes, setup-secret hardening, and container dependency remediation.
8398
- `scripts/run-container-setup.py` - Uses the same secure writer for normal,
8499
confirmed-overwrite, and forced setup paths.
85100
- `scripts/create_code_scanning_combined_serif.py` - Replaces a polynomial
86-
backtracking Link-header expression with a bounded linear parser while
87-
preserving GitHub pagination relations.
101+
backtracking Link-header expression with a bounded linear parser and confines
102+
API requests, pagination, redirects, and downloads to the configured exact
103+
HTTPS origin.
88104
- `scripts/create-demo-user.py` - Stops printing the demo password, generated
89105
API/public keys, encrypted password, and secret-bearing exception payloads.
90106
- `scripts/incoming_clean_up.py` - Adds an explicit trusted filesystem boundary,
91-
canonical containment before every access/mutation, symlink-safe destructive
92-
operations, full ZIP-member preflight/extraction, and traversal-safe SFV
93-
verification and renaming.
107+
canonical containment before every access/mutation, pinned descriptor-relative
108+
no-follow operations, `renameat2(RENAME_NOREPLACE)` quarantine publication,
109+
full ZIP-member preflight/extraction, and traversal-safe SFV verification and
110+
renaming. Live cleanup fails closed where the required Linux/POSIX primitives
111+
are unavailable; extracted directories and files are private `0700` and
112+
`0600` objects respectively.
94113
- `scripts/tests/test_setup_melodee.py` - Covers secret generation, POSIX
95114
permissions, existing-file preservation, malformed templates, symlink
96115
refusal, and non-regular destinations.
@@ -103,6 +122,19 @@ fixes, setup-secret hardening, and container dependency remediation.
103122
- `.github/codeql/codeql-config.yml` - Removes invalid model configuration and
104123
broad rule exclusions while retaining the default remote threat model for
105124
the compiled web application.
125+
- `src/Melodee.Common/Services/DeviceIdentificationService.cs`,
126+
`src/Melodee.Common/Services/PlaylistImportService.cs`,
127+
`src/Melodee.Common/Services/ScriptEvaluation/ScriptAdminService.cs`,
128+
`src/Melodee.Common/Services/SearchEngines/ArtistSearchEngineService.cs`, and
129+
`src/Melodee.Common/Services/SettingService.cs` - Sanitize the six
130+
attacker-controlled fields found by the fresh default-remote C# log-forging
131+
scan.
132+
- `src/Melodee.Common/Services/Playback/Backends/MpdPlaybackBackend.cs` - Keeps
133+
the MPD password command on the wire while passing only a credential-free
134+
command representation to every log path.
135+
- `tests/Melodee.Tests.Common/Services/Playback/PlaybackBackendTests.cs` -
136+
Verifies that password authentication still reaches MPD and never reaches
137+
application logs.
106138
- `.github/CODEQL-WORKFLOW.md` - Documents the single advanced setup, model
107139
behavior, verification, and stale GitHub configuration cleanup.
108140
- `Dockerfile` - Moves the shipped runtime to the official .NET 10 Ubuntu 26.04
@@ -136,3 +168,75 @@ fixes, setup-secret hardening, and container dependency remediation.
136168
pack.
137169
- `.github/docker/Dockerfile.container-scan` - Removed the reduced scan image
138170
so CI analyzes the same final image that releases publish.
171+
172+
## Verified Results
173+
174+
### CodeQL
175+
176+
- The six live baseline C# password-reset privacy flows have source fixes. The
177+
Python setup-secret alert is handled by atomic owner-only persistence,
178+
regression coverage, and one narrow documented suppression for the necessary
179+
`.env` sink.
180+
- A fresh C# database using the default remote threat model moved from eight
181+
findings to zero after fixing six log-forging paths and two MPD
182+
password-to-log flows.
183+
- Fresh GitHub Actions and JavaScript/TypeScript databases each report zero
184+
findings.
185+
- Restored local Python analysis identified the setup persistence boundary,
186+
polynomial Link parsing, secret-bearing output, filesystem/ZIP/SFV paths, and
187+
same-origin exporter request handling. A fresh workflow-equivalent database
188+
reports zero findings and no SARIF warning/error notifications across all 45
189+
default queries. The 52-query security-extended suite also reports zero
190+
findings.
191+
192+
The shared configuration keeps the compiled C# application, JavaScript, and
193+
Actions on CodeQL's default remote trust boundary. Python alone enables the
194+
local threat model because maintenance scripts intentionally consume command
195+
line arguments, environment variables, and local files. This avoids treating
196+
ordinary application-owned database and filesystem state as remotely
197+
attacker-controlled while preserving the stricter model where it represents
198+
the Python tools' real inputs.
199+
200+
### Container and Dependency Inventory
201+
202+
- Trivy 0.72.0 reduced raw production-image findings from 416 to 140: 276
203+
removed (66.3%). The remaining inventory represents 41 CVEs, split into 124
204+
medium and 16 low findings.
205+
- The remaining image inventory contains zero critical, high, fixable,
206+
.NET-package, or application-package findings.
207+
- GitHub receives only the intended critical/high SARIF policy. CI retains the
208+
complete all-severity JSON report for review and future fix-availability
209+
changes.
210+
- NuGet reports zero vulnerable dependencies.
211+
212+
### Build and Runtime Verification
213+
214+
- The complete solution builds with zero warnings and errors.
215+
- The full .NET suite passes 5,885 tests with 34 skipped and zero failed.
216+
- All 109 Python script tests pass with `ResourceWarning` promoted to an error,
217+
including 57 focused incoming-cleanup security and race regressions.
218+
- Jekyll, `actionlint`, YAML parsing, shell syntax/static analysis, and Docker
219+
configuration checks pass.
220+
- A real PostgreSQL integration starts the production image with the
221+
unprivileged `melodee` process as PID 1, reaches healthy status, and confirms
222+
that configured database/authentication secrets are absent from container
223+
logs.
224+
225+
## Release Summary
226+
227+
The local release gate is complete: all four CodeQL languages report zero fresh
228+
findings, the production image contains no critical, high, or currently fixable
229+
inventory, the .NET and Python suites pass, and the production container passes
230+
its real-database non-root integration. The remaining 140 raw image findings
231+
are quantified medium/low Ubuntu-package issues without available fixes.
232+
233+
## Post-Merge Verification
234+
235+
- The final pre-merge API query still reports the 423-alert `main` baseline.
236+
This is expected because GitHub has not analyzed this branch; no server-side
237+
alert closure is claimed by the local results.
238+
- After the checked-in Python job completes successfully on `main`, delete the
239+
stale `dynamic/github-code-scanning/codeql:analyze` and
240+
`dynamic/github-code-scanning/codeql:upload` Python configurations from
241+
**Security and quality > Code scanning > Tool status**. Retain
242+
`.github/workflows/codeql.yml:analyze`.

design/docs/20260711-code-scanning-remediation-plan.md

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,14 @@ risk or removing required Melodee runtime features.
1313

1414
## Baseline
1515

16-
- CodeQL: seven alerts
16+
- Total: 423 open code-scanning alerts on `main`.
17+
- CodeQL: seven alerts.
1718
- Six `cs/exposure-of-sensitive-information` findings in the forgot-password
1819
page.
1920
- One `py/clear-text-storage-sensitive-data` finding in the setup script.
2021
- Trivy: 416 alerts in the `library/melodee` container image.
21-
- All current Trivy findings are medium or low severity.
22+
- The baseline contained 382 medium and 34 low findings, with no critical or
23+
high findings.
2224
- Most findings come from the Ubuntu FFmpeg 6.1 package and its shared
2325
libraries.
2426

@@ -33,8 +35,8 @@ risk or removing required Melodee runtime features.
3335

3436
- [x] Remove sensitive-information exposure from the forgot-password flow
3537
while retaining enumeration resistance and useful diagnostics.
36-
- [x] Remove clear-text password persistence from the Python setup flow while
37-
preserving interactive and unattended setup behavior.
38+
- [x] Harden the Python setup flow's necessary `.env` persistence with atomic,
39+
owner-only writes and document the narrow unavoidable CodeQL sink.
3840
- [x] Add or update focused regression tests for both flows.
3941
- [x] Consolidate GitHub Actions, C#, JavaScript/TypeScript, and Python under
4042
the advanced CodeQL workflow and replace broad query exclusions with precise
@@ -45,6 +47,11 @@ risk or removing required Melodee runtime features.
4547
exception payloads from adjacent authentication and SMTP logs.
4648
- [x] Remediate the ReDoS, demo-secret logging, and filesystem path-injection
4749
findings exposed by the newly enabled local Python CodeQL scan.
50+
- [x] Restrict the GitHub code-scanning exporter to bounded, same-origin HTTPS
51+
requests and redirects so pagination cannot become an SSRF path.
52+
- [x] Reduce a fresh default-remote C# scan from eight findings to zero by
53+
sanitizing six log-forging flows and separating MPD wire commands from their
54+
credential-free log representation.
4855

4956
## Phase 3: Container Remediation
5057

@@ -57,20 +64,51 @@ risk or removing required Melodee runtime features.
5764
- [x] Generate a local post-change Trivy inventory and quantify remaining
5865
findings.
5966

60-
## Phase 4: Verification and Release Notes
67+
## Phase 4: Verification and Release Notes (Complete)
6168

62-
- [ ] Run focused tests for each security fix.
63-
- [ ] Build the production container and smoke-test required executables and
69+
- [x] Run focused tests for the completed C#, setup, workflow, and container
70+
fixes.
71+
- [x] Build the production container and smoke-test required executables and
6472
non-root startup behavior.
65-
- [ ] Run the full .NET build and test suite without warnings.
66-
- [ ] Validate Python, YAML, shell, and Docker configuration changes.
67-
- [ ] Update the public changelog and complete the release change record.
73+
- [x] Run the full .NET build and test suite without warnings.
74+
- [x] Complete the Python filesystem audit, its final focused tests, and
75+
a fresh full-query Python CodeQL scan with the local threat model.
76+
- [x] Validate Jekyll, GitHub Actions, YAML, shell, and Docker configuration
77+
changes.
78+
- [x] Update the public changelog and release change record with the final
79+
verified results.
80+
81+
## Verification Snapshot
82+
83+
- Six original C# CodeQL alerts have source fixes and regression coverage. The
84+
Python setup alert is handled by a hardened, necessary persistence boundary,
85+
regression coverage, and one narrow documented suppression.
86+
- Fresh local CodeQL scans report C# at `8 -> 0`, GitHub Actions at zero, and
87+
JavaScript/TypeScript at zero under the default remote threat model.
88+
- A fresh workflow-equivalent local-threat-model Python database reports zero
89+
findings and no SARIF warning/error notifications across all 45 default
90+
queries. The 52-query security-extended suite also reports zero findings.
91+
- The Python test run passes all 109 script tests with `ResourceWarning`
92+
promoted to an error, including all 57 focused cleanup filesystem, ZIP, SFV,
93+
shutdown, and race regressions.
94+
- Trivy 0.72.0 reduced the production-image inventory from 416 to 140 raw
95+
findings: 276 removed (66.3%), 41 remaining CVEs, 124 medium and 16 low.
96+
None are critical, high, fixable, or attributable to .NET/application
97+
packages.
98+
- The solution builds with zero warnings. The full .NET suite passes 5,885
99+
tests with 34 skipped, and NuGet reports zero vulnerable dependencies.
100+
- Jekyll, `actionlint`, YAML, shell, and container checks pass. A real
101+
PostgreSQL container integration confirmed health, non-root PID 1 startup,
102+
and that configured secret values do not appear in logs.
68103

69104
## Success Criteria
70105

71-
- All seven current CodeQL data-flow findings have source fixes and regression
72-
coverage or a documented, evidence-backed reason they remain.
106+
- All seven baseline CodeQL data-flow findings have either a source fix or a
107+
narrowly documented, security-hardened necessary persistence boundary, with
108+
regression coverage.
73109
- The container alert count is materially reduced without suppressing fixed or
74110
high-impact vulnerabilities.
75111
- The solution and container continue to build and required tests pass.
76112
- Remaining alerts are quantified by severity, package, and fix availability.
113+
- The Python audit and local-threat-model scans are complete; GitHub alert
114+
reconciliation remains a post-merge administrative step.

0 commit comments

Comments
 (0)