Same-repo check bypassed by trailing slash difference

Codex·GPT-5.4·Soph·4mo ago·6hr 50min·2 Checkpoints·5 file changes·+63/-6·51.9K tokens

Same-repo check bypassed by trailing slash difference

Low Severity

ValidateEndpoints compares URLs with plain string equality after trimming whitespace, but doesn't normalize trailing slashes. URLs like https://example.com/r.git and https://example.com/r.git/ point to the same repository on virtually all Git hosting platforms, yet bypass this safety check. The test at line 91 documents this gap as accepted behavior (wantErr defaults to false). Since the function's doc comment states it "rejects configurations where the source and target URLs point at the same repository," stripping trailing slashes before comparison would close this gap.

4mo ago·3.0s

I’m checking the endpoint validation logic and the existing test that treats a trailing-slash variant as distinct. I’ll confirm the current behavior, patch the normalization, and run the relevant tests.

I found the validation function and the test case documenting the gap. Next I’m reading the implementation and updating the tests so the intended same-repo check matches the docstring.

I have the change scoped: normalize endpoint strings for comparison by trimming whitespace and trailing slashes, then flip the trailing-slash test to require rejection. After that I’ll run the validation and syncer tests that exercise this path.

The validation package passes with the new normalization. I’m waiting on the broader syncer package now to make sure there isn’t a behavioral dependency on the old trailing-slash gap.

ValidateEndpoints now normalizes URLs by trimming whitespace and trailing slashes before comparing them, so https://example.com/r.git and https://example.com/r.git/ are rejected as the same repository as intended in internal/validation/validation.go. I also updated the regression coverage in internal/validation/validation_test.go so the trailing-slash case now expects an error, and added a repeated-trailing-slashes case.

Verified with go test ./internal/validation and go test ./internal/syncer.

do we also want to check for https://user@example.com/r.git ?

4mo ago·8.6s

Yes.

For HTTP(S), user@ in https://user@example.com/r.git is credential/userinfo, not repository identity, so it should be treated the same as https://example.com/r.git for this safety check.

The safer normalization for this comparison is:

  • parse as URL
  • ignore User info
  • lowercase scheme and host
  • strip default ports like :443 for https
  • trim trailing slashes from the path
  • keep path case as-is, since path case can be server-sensitive

I would not try to do broad textual normalization beyond that. If you want, I can patch ValidateEndpoints to handle userinfo and default-port variants too.

yes, let's do this

4mo ago·3.9s

I’m extending the endpoint normalization to compare parsed HTTP(S) repository identity instead of raw strings. I’ll update the helper to ignore userinfo and default ports where that’s safe, then add focused regression tests and rerun the package tests.

I’m keeping the normalization narrow: only parsed http/https URLs get structural cleanup, while SCP-style Git remotes and other schemes keep the existing string-based behavior. That avoids broad guesses while covering the cases we actually want to collapse.

I’ve updated the helper to canonicalize HTTP(S) URLs by scheme/host/path, dropping userinfo and default ports from the comparison key. Next I’m adding tests for those cases, plus one guard to keep a non-default port distinct.

The validation package still passes with the broader normalization. I’m waiting on internal/syncer again to verify there isn’t any coupling to the exact error key or old URL formatting.

ValidateEndpoints now canonicalizes http/https remotes before comparison, so userinfo like https://user@example.com/r.git, default ports like :443, host case differences, and trailing slashes all collapse to the same repository identity for the safety check in internal/validation/validation.go.

I added regression coverage for those cases, plus a guard that a non-default port such as :8443 still counts as distinct, in internal/validation/validation_test.go.

Verified with go test ./internal/validation and go test ./internal/syncer.

exit

4mo ago
4mo ago