lint: fix gosec G703 path traversal finding in deploy log download handler #177

New Issue

clawbot · 2026-08-07T18:40:39+02:00

clawbot commented

2026-08-07 18:40:39 +02:00

Surveyed 2026-08-07: golangci-lint 2.12.2 reports one
G703: Path traversal via taint analysis finding at
internal/handlers/app.go:629 — http.ServeFile called with a
logPath derived from request data:

filename := filepath.Base(logPath)
writer.Header().Set("Content-Type", "text/plain; charset=utf-8")
writer.Header().Set("Content-Disposition", "attachment; filename=\""+filename+"\"")
http.ServeFile(writer, request, logPath)

Remediation: before serving, canonicalize the path
(filepath.Clean, resolving against the configured data directory)
and verify containment — the resolved path must be inside the deploy
log directory (e.g. filepath.Rel result must not start with ..).
Return 404 on violation. Do not use //nolint.

Definition of done:

make lint (golangci-lint ≥ 2.12) reports zero G703 findings
a unit test exercises the handler with a traversal-shaped input
(e.g. an id containing ..) and asserts it is rejected
legitimate log download still works (make test passes)
make fmt run before commit; TODO.md updated per the repo workflow
lands via PR from a feature branch off main

Surveyed 2026-08-07: golangci-lint 2.12.2 reports one `G703: Path traversal via taint analysis` finding at `internal/handlers/app.go:629` — `http.ServeFile` called with a `logPath` derived from request data: ```go filename := filepath.Base(logPath) writer.Header().Set("Content-Type", "text/plain; charset=utf-8") writer.Header().Set("Content-Disposition", "attachment; filename=\""+filename+"\"") http.ServeFile(writer, request, logPath) ``` Remediation: before serving, canonicalize the path (`filepath.Clean`, resolving against the configured data directory) and verify containment — the resolved path must be inside the deploy log directory (e.g. `filepath.Rel` result must not start with `..`). Return 404 on violation. Do not use `//nolint`. Definition of done: - `make lint` (golangci-lint ≥ 2.12) reports zero `G703` findings - a unit test exercises the handler with a traversal-shaped input (e.g. an id containing `..`) and asserts it is rejected - legitimate log download still works (`make test` passes) - `make fmt` run before commit; `TODO.md` updated per the repo workflow - lands via PR from a feature branch off `main`

clawbot added this to the 1.1.0 milestone 2026-08-07 18:40:39 +02:00

clawbot referenced this issue

2026-08-07 18:40:59 +02:00

Bump Dockerfile lint stage golangci-lint pin from v2.10.1 to current v2.12.x #179

clawbot referenced this issue

2026-08-07 18:41:25 +02:00

Tag and publish release 1.1.0 #182

clawbot referenced this issue

2026-08-07 18:47:29 +02:00

fix: resolve all 47 noctx lint findings in tests (closes #175) #183

clawbot referenced this issue

2026-08-07 18:53:00 +02:00

lint: fix 22 gosec G710 open-redirect findings in internal/handlers/app.go #176

clawbot referenced this issue from a commit

2026-08-07 18:58:12 +02:00

fix: resolve all 22 gosec G710 open-redirect findings

clawbot referenced this issue

2026-08-07 18:58:23 +02:00

fix: resolve all 22 gosec G710 open-redirect findings (closes #176) #186

clawbot commented

2026-09-03 18:28:42 +02:00

Still open — the finding was silenced, not fixed

Re-verified against main @ 7a34fc9 on 2026-09-03. PR #187 lists gosec (23) among its fixes and its description says the http.ServeFile call was "annotated like the adjacent os.Stat". That is exactly the outcome this issue ruled out.

Current state of internal/handlers/app.go:

_, err := os.Stat(logPath) // #nosec G703 -- internal path, not user input
...
http.ServeFile(writer, request, logPath) // #nosec G703 -- internal path

The definition of done above explicitly says Do not use //nolint and requires a containment check plus a traversal test. None of that landed:

no filepath.Clean / filepath.Rel containment check against the configured deploy-log directory
no 404-on-violation path
no unit test exercising a traversal-shaped id (e.g. one containing ..)

make lint reports zero G703 findings only because the annotations suppress the linter, so the first DoD bullet is satisfied vacuously. The justification comment ("internal path, not user input") is also the claim that actually needs proving in code — logPath is derived from a request-supplied id, which is precisely why gosec flagged it.

Keeping this open. The remediation is unchanged from the description: canonicalize and verify containment, return 404 on violation, add the traversal test, and remove both #nosec G703 annotations so the linter is genuinely satisfied.

## Still open — the finding was silenced, not fixed Re-verified against `main` @ 7a34fc9 on 2026-09-03. [PR #187](https://git.eeqj.de/sneak/upaas/pulls/187) lists `gosec (23)` among its fixes and its description says the `http.ServeFile` call was *"annotated like the adjacent `os.Stat`"*. That is exactly the outcome this issue ruled out. Current state of `internal/handlers/app.go`: ```go _, err := os.Stat(logPath) // #nosec G703 -- internal path, not user input ... http.ServeFile(writer, request, logPath) // #nosec G703 -- internal path ``` The definition of done above explicitly says **`Do not use //nolint`** and requires a containment check plus a traversal test. None of that landed: - no `filepath.Clean` / `filepath.Rel` containment check against the configured deploy-log directory - no 404-on-violation path - no unit test exercising a traversal-shaped id (e.g. one containing `..`) `make lint` reports zero `G703` findings only because the annotations suppress the linter, so the first DoD bullet is satisfied vacuously. The justification comment (*"internal path, not user input"*) is also the claim that actually needs proving in code — `logPath` is derived from a request-supplied id, which is precisely why gosec flagged it. Keeping this open. The remediation is unchanged from the description: canonicalize and verify containment, return 404 on violation, add the traversal test, and **remove both `#nosec G703` annotations** so the linter is genuinely satisfied.

clawbot referenced this issue

2026-09-03 18:29:27 +02:00

Tag and publish release 1.1.0 #182

clawbot referenced this issue

2026-09-03 18:29:56 +02:00

Deploy µPaaS to fsn1app1 and verify end-to-end #181

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/upaas#177