mosaicstack/stack
1
0
Fork 0
Code Issues 85 Pull Requests Actions Packages Projects Releases 15 Wiki Activity

fix(tools/git): pr-review.sh _belongs case-sensitive path compare rejects mixed-case repo slugs #875

New Issue
Closed
opened 2026-07-23 16:49:26 +00:00 by jason.woltje · 0 comments
jason.woltje commented 2026-07-23 16:49:26 +00:00
Owner
Copy Link

Follow-up to #866 (must land AFTER #866 merges). #866 hardened the pr-review.sh comment-persistence readback into a full-path _belongs() check (pins scheme+host+port origin + FULL path, to block look-alike-host / decoy-prefix attacks). But _origin_and_path() lowercases scheme+host and NOT the path, and _belongs compares path == expected_path CASE-SENSITIVELY; expected_pr_path = base_path/{slug}/pulls/{n} embeds the repo slug from GITEA_API_BASE (may be mixed-case, e.g. USC/uconnect) while Gitea canonicalizes the returned pull_request_url slug to lowercase. Result: a successful post (201) false-negatives the readback (repository/PR mismatch). It fails CLOSED (reports failure on a real success, never a false-success) so it is not a security hole, but it breaks legit mixed-case-slug repos. FIX: lowercase the slug+path in the _belongs comparison (repo slugs are case-insensitive/canonicalized lowercase by Gitea), PRESERVING the scheme+host+port origin pin from #866. Supersedes the OLD-code fix in the Mos tooling-patch kit (Patch 4), which was written against main's pre-#866 endswith() code and no longer applies. depends-on: #866.

Follow-up to #866 (must land AFTER #866 merges). #866 hardened the pr-review.sh comment-persistence readback into a full-path _belongs() check (pins scheme+host+port origin + FULL path, to block look-alike-host / decoy-prefix attacks). But _origin_and_path() lowercases scheme+host and NOT the path, and _belongs compares path == expected_path CASE-SENSITIVELY; expected_pr_path = base_path/{slug}/pulls/{n} embeds the repo slug from GITEA_API_BASE (may be mixed-case, e.g. USC/uconnect) while Gitea canonicalizes the returned pull_request_url slug to lowercase. Result: a successful post (201) false-negatives the readback (repository/PR mismatch). It fails CLOSED (reports failure on a real success, never a false-success) so it is not a security hole, but it breaks legit mixed-case-slug repos. FIX: lowercase the slug+path in the _belongs comparison (repo slugs are case-insensitive/canonicalized lowercase by Gitea), PRESERVING the scheme+host+port origin pin from #866. Supersedes the OLD-code fix in the Mos tooling-patch kit (Patch 4), which was written against main's pre-#866 endswith() code and no longer applies. depends-on: #866.
Mos closed this issue 2026-07-23 17:53:27 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
jason.woltje
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#875
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?