investigate: confirm the path-escaping guard covers every internal/gitea builder after #609 #850

New issue

Open

opened 2026-10-20 17:20:11 +00:00 by hexajon · 0 comments

hexajon commented

2026-10-20 17:20:11 +00:00

Owner

What happens

#609 set the rule that every value going into an API path or query is escaped, and it added a guard test that fails when a new builder joins a variable into a path without escaping it. #609 is closed. But #711, filed later, found the repo-flags builder still interpolated raw segments with fmt.Sprintf, so it had slipped past #609's sweep and its guard. #711 is also closed. That one escape raises the exact question #609's guard was meant to settle: does the guard catch every request-path builder, or can another one still slip the way repo-flags did?

Why it matters

The original audit listed a raw-path-concat inventory of about 13 builders. #609 states the rest already route through escapePath, but repo-flags proves at least one slipped. A guard that misses a builder gives false confidence. This is same-origin path reshaping only, so the repo-scope boundary holds and the severity is low, but a guard that does not actually guard is worth confirming. No decision record governs the path-builder escaping itself; it is tracked in #609 and #711, so this investigation changes no ADR.

Investigate

Enumerate every request-path and query builder in internal/gitea and check each escapes its variable segments (through apiPath, escapePath, or url.Values).
Check whether #609's guard test inspects every builder or only a subset, so a missed or new builder, as repo-flags was, is caught.
If a builder is still raw, route it through the escaper. If the guard has a gap, widen it. If every builder is already escaped and guarded, record that the inventory is closed.

Acceptance criteria

Every internal/gitea request-path and query builder is confirmed to escape its variable segments, or the raw one is fixed.
The guard test is confirmed to fail for any builder that joins an unescaped variable into a path, or it is widened until it does.
The outcome is recorded: the inventory is closed, or the gap and its fix are named.

#609 (escape every value into an API path or query, closed), #711 (repo-flags builder, closed), #732. Severity: low (same-origin path reshaping; the repo-scope boundary is unaffected).

## What happens #609 set the rule that every value going into an API path or query is escaped, and it added a guard test that fails when a new builder joins a variable into a path without escaping it. #609 is closed. But #711, filed later, found the repo-flags builder still interpolated raw segments with `fmt.Sprintf`, so it had slipped past #609's sweep and its guard. #711 is also closed. That one escape raises the exact question #609's guard was meant to settle: does the guard catch every request-path builder, or can another one still slip the way repo-flags did? ## Why it matters The original audit listed a raw-path-concat inventory of about 13 builders. #609 states the rest already route through `escapePath`, but repo-flags proves at least one slipped. A guard that misses a builder gives false confidence. This is same-origin path reshaping only, so the repo-scope boundary holds and the severity is low, but a guard that does not actually guard is worth confirming. No decision record governs the path-builder escaping itself; it is tracked in #609 and #711, so this investigation changes no ADR. ## Investigate - Enumerate every request-path and query builder in `internal/gitea` and check each escapes its variable segments (through `apiPath`, `escapePath`, or `url.Values`). - Check whether #609's guard test inspects every builder or only a subset, so a missed or new builder, as repo-flags was, is caught. - If a builder is still raw, route it through the escaper. If the guard has a gap, widen it. If every builder is already escaped and guarded, record that the inventory is closed. ## Acceptance criteria - Every `internal/gitea` request-path and query builder is confirmed to escape its variable segments, or the raw one is fixed. - The guard test is confirmed to fail for any builder that joins an unescaped variable into a path, or it is widened until it does. - The outcome is recorded: the inventory is closed, or the gap and its fix are named. ## Related - #609 (escape every value into an API path or query, closed), #711 (repo-flags builder, closed), #732. Severity: low (same-origin path reshaping; the repo-scope boundary is unaffected).

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

sixfold-space/madtea#850

No description provided.

Rows
Columns