Harden development tools for CodeQL path and log injection findings#23
Merged
Conversation
Copilot stopped work on behalf of
JediRhymeTrix due to an error
April 27, 2026 17:58
JediRhymeTrix
added a commit
that referenced
this pull request
Apr 27, 2026
`copyFile` in `internal/adapters/moon/capture.go` used `defer out.Close()`, silently discarding close errors on the writable output file. Buffered filesystem write errors surface only at close time, so this could corrupt fixture captures without any signal. Resolves CodeQL alert `go/unhandled-writable-file-close` (#23). ## Changes - **`internal/adapters/moon/capture.go`** — Rewrote `copyFile` with a named return and an explicit deferred close that propagates the close error only when `io.Copy` succeeded, preserving the copy error otherwise: ```go func copyFile(src, dst string) (retErr error) { // ...open in/out... defer func() { if cerr := out.Close(); cerr != nil && retErr == nil { retErr = cerr } }() _, retErr = io.Copy(out, in) return retErr } ``` `captureUpload` semantics are unchanged — failures are logged and never propagated to WebDAV clients. - **`internal/adapters/moon/capture_test.go`** *(new, no CGO)* — Three unit tests covering: successful copy, missing source, and `O_EXCL` destination-already-exists semantics. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JediRhymeTrix <26216205+JediRhymeTrix@users.noreply.github.com>
Copilot stopped work on behalf of
JediRhymeTrix due to an error
April 27, 2026 21:11
Agent-Logs-Url: https://github.com/JediRhymeTrix/readsync/sessions/7fce9f10-aa4e-47a7-8fc4-b9d2a9f7868f Co-authored-by: JediRhymeTrix <26216205+JediRhymeTrix@users.noreply.github.com>
JediRhymeTrix
force-pushed
the
copilot/harden-dev-tools-for-codeql
branch
from
April 27, 2026 22:49
37911bf to
11589a9
Compare
Owner
|
@copilot rebase and retry |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
Author
Rebased onto latest |
Owner
|
@copilot address CodeQL findings |
…ak CodeQL taint chains
Replace strings.Map with strings.NewReplacer (the pattern CodeQL's go/log-injection
query recognizes as a sanitizer) in both moon-fixture-recorder and koreader-sim.
Replace fmt.Sprintf("% x", body[:...]) with encoding/hex.EncodeToString so the
magic-bytes log line no longer carries taint from user-controlled HTTP body bytes.
Update test expectations to match the new \n/\r → literal-escape behavior.
Agent-Logs-Url: https://github.com/JediRhymeTrix/readsync/sessions/fc8c3aa4-d283-419f-9c54-e280649d9402
Co-authored-by: JediRhymeTrix <26216205+JediRhymeTrix@users.noreply.github.com>
Contributor
Author
Addressed in
Test expectations updated to match the new |
Copilot
AI
changed the title
[WIP] Harden development tools for CodeQL path and log injection findings
Harden development tools for CodeQL path and log injection findings
Apr 27, 2026
JediRhymeTrix
marked this pull request as ready for review
April 27, 2026 23:18
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hardens the Phase 0 development tools against CodeQL
go/path-injectionandgo/log-injectionfindings.Changes Made
tools/moon-fixture-recorder/recorder.gosanitizeLog()— implemented usingstrings.NewReplacer("\n", "\\n", "\r", "\\r"), the sanitizer pattern CodeQL'sgo/log-injectionquery recognizes as a taint barrier; replaces CR/LF with their visible escape sequences rather than spacessanitizeFilename()— allowlist-only (alphanumeric,-,_,.) safe capture filenamessafeDiskPath()— replacesdiskPath(); rejects..components and backslashes, verifies the resolved path stays underdavRootvia abs-prefix check; returns(string, error)safeDiskPath()callers updated to return HTTP 400 on invalid pathssaveCapturedPO()updated to usepath.Base+sanitizeFilenamelog.Printfcalls usingr.URL.Pathwrapped withsanitizeLog()fmt.Sprintf("% x", body[:...])withencoding/hex.EncodeToString(body[:...])— output constrained to[0-9a-f], eliminating the taint path from user-controlled HTTP body bytes (CodeQL alert #29)tools/koreader-sim/handlers.gosanitizeLog()— samestrings.NewReplacerpatternlogfcalls using user-controlledu,req.Device, anddocHashwrapped withsanitizeLog()Tests Added
tools/moon-fixture-recorder/recorder_test.go: path traversal rejection, log sanitization (escape-sequence behavior), filename sanitization, and capture containment teststools/koreader-sim/handlers_test.go: log sanitization tests covering CR/LF injection attemptsTesting
go test -count=1 ./tools/moon-fixture-recorder/ ./tools/koreader-sim/— all tests pass