Skip to content

fix(release): validate GitHub remote identity - #17

Open
alectimison-maker wants to merge 1 commit into
loadingalias:mainfrom
alectimison-maker:fix/github-remote-parsing
Open

fix(release): validate GitHub remote identity#17
alectimison-maker wants to merge 1 commit into
loadingalias:mainfrom
alectimison-maker:fix/github-remote-parsing

Conversation

@alectimison-maker

Copy link
Copy Markdown
Contributor

Summary

  • normalize trailing slashes before removing the optional .git suffix from GitHub remotes
  • reject empty identities, extra path segments, queries, and fragments instead of deriving a misleading repository
  • add regression tests and a patch-level release change file

Why

Automatic repository detection feeds changelog links and exact-SHA GitHub release readiness. A remote such as https://github.com/org/repo.git/ was previously parsed as repository repo.git, while a non-repository path such as https://github.com/org/repo/issues was silently accepted as org/repo.

Compatibility

The existing HTTPS, SCP-like SSH, and ssh:// forms remain supported. URLs with extra non-repository path/query/fragment data now fail closed, allowing the existing no-auto-link fallback to apply.

Verification

  • red/green regression run: cargo +stable test --ignore-rust-version --lib parse_github_remote
  • cargo +stable fmt --all -- --check
  • cargo +stable clippy --ignore-rust-version --all-targets --all-features -- -D warnings
  • cargo +stable doc --ignore-rust-version --workspace --no-deps --all-features --locked with RUSTDOCFLAGS=-D warnings
  • cargo +stable deny check all
  • cargo +stable audit --no-fetch
  • cargo +stable nextest run --workspace -P default --all-features --locked --config-file .config/nextest.toml --ignore-rust-version

The local host has Rust 1.93.1, so the MSRV gate was explicitly bypassed for local code-path testing. The repository's Rust 1.95 compatibility matrix remains authoritative. The Windows nextest run completed the full inventory but reported host/resource-specific failures detailed in the PR check results; the changed parser tests passed.

@loadingalias

Copy link
Copy Markdown
Owner

Hey, @alectimison-maker. I'm sorry it's taken me so long to get to this. Forgive me, it's not really how I work. I just can't let the robots take the PRs over. I have to look the over myself and I was deep into a significant update when this landed.

So, yeah, great find. Thank you. The PR was sound in theory/impl: normalize trailing slashes before removing .git, and accept only exact GitHub owner/repository paths.

I've made significant improvements (caching, etc) and cleared out some indirection (removing the 'run' workflows) for v0.22. That left this PR unmergable and I didn't expect you to know what I'd done. So, I've implemented a small fix to integrate it.

I incorporated your fix into 012e7fd (012e7fd) and extended the same repo identity invariant across the complete release path:

• normalize common Git transport forms into one exact identity;
• reject ambiguous, malformed, or divergent fetch/push repositories;
• persist and revalidate the identity during release recovery;
• explicitly target every gh and glab operation;
• preserve local aborts before any remote effect;
• cover parsing, drift, recovery, and forge targeting with regression tests

Your original change intent is preserved, and you are credited as a co-author on the landed commit. This PR is now superseded by the implementation on main. Thank you for catching this and providing the initial fix & regression coverage. I appreciate it!

The v0.22 is running final checks/tests and will be released shortly. Thanks again!

@alectimison-maker

Copy link
Copy Markdown
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants