## PR #66 Overview

### Pushed

Commit `e7e4a1a` is on `nodo/convert-sha256`.

### Comments Addressed

| # | Reviewer      | Comment                                                | Resolution                                                                                                                                                                                                                                                                                     |
|---|---------------|-------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| 1 | cursor[bot]   | Discovery ignores ctx cancel                           | Already fixed — `discoverReachable` rewrite (iterative DFS checking `ctx.Err()` per object) predates this; bot reviewed an old commit. Reply posted.                                                                                                                                     |
| 2 | pjbgf         | objfile no longer hardcodes SHA1 in alpha.4          | **Refactored** to `dst.NewEncodedObject` + `SetEncodedObject`; removed `encodeBody`/`writeLoose`. Doc note updated.                                                                                                                                                                       |
| 3 | pjbgf         | prefer `--sign-mode` over `--sign`                    | **Renamed** `--sign` → `--sign-mode {none,tips}` (default `none`), validated in `Run`, room for a future `all`.                                                                                                                                                                            |
| 4 | pjbgf         | concerns signing tags in place?                       | Explained the premise (branch tips aren't signable objects → wrapped in `converted/<branch>` tags; in-place tag signing is the future `all` mode). Reply posted.                                                                                                                            |

### Verification

- `go build ./...`, `go test ./...` — clean
- Gated `TestRun_GitHTTPBackend` + `TestRun_GitHTTPBackend_Sign` (`GITSYNC_E2E_SHA256_HTTP_BACKEND=1`) — pass; full conversion via `SetEncodedObject` produces an `fsck`-clean SHA256 repo, and `--sign-mode tips` signs as before
- `go test -race` — clean
- `golangci-lint` — 0 issues
- The on-disk `sha256(content)` invariant test still passes against the new write path, kept as a regression guard.

One judgment call worth flagging: the refactor changes loose-object compression from `zlib.BestSpeed` (level 1) to go-git's default (level 6), so very large conversions will spend somewhat more CPU compressing — the trade-off you opted into by choosing the refactor. If that ever bites on kernel-scale repos, `git gc` on the output packs it away regardless.

---

### Additional Comments

```go
I think we could be leaking credentials here:
    msg := fmt.Sprintf(
        "SHA1 → SHA256 conversion attestation for %s.\n\n"+
        "Source: %s\nProduced by git-sync convert-sha256.\n",
        refName, sourceURL)
```

### Credential Leak — Fixed

The source URL can carry `user:token@host`, and it was being copied verbatim into three surfaces, the worst being the **signed attestation tag message** (permanent and pushed). Also the `fetching …` status line and the `SourceURL` field in both the text and JSON result.

Fix: a `redactSourceURL` helper strips the **entire userinfo** component before any of those surfaces. I deliberately did _not_ use Go's `url.URL.Redacted()` — it only masks the password, so a token-in-username form like `https://ghp_xxx@github.com/...` (common for GitHub token auth) would survive untouched. The fetch path keeps the original `req.SourceURL`, so auth is unaffected.

Covered by a new `TestRedactSourceURL` (plain creds, token-as-username, x-access-token, and the unparseable-input → placeholder path). Full suite + gated E2E/signing tests + lint + gofmt all green.

---

### Foreign Pull/Merge-Request Refs Handling

```text
All code in a pull ref is foreign to the repository before a merge. Bringing that in and pushing to a different forge which likely has different semantics around refs/pull can be used to deliver malicious code from the repo target.
```

### Change Complete and Clean

The change is complete and clean. Let me commit and push:

- refs/pull/* (GitHub/Gitea), refs/pull-requests/* (Bitbucket), and refs/merge-requests/* (GitLab) hold code proposed from forks and other branches — foreign to the repository until merged. --all-refs pulled them in, and since the converted repo is typically mirrored onward with `git push --mirror`, a destination forge that doesn't treat those namespaces as read-only PR refs could republish unreviewed code as repo content.

**Exclude those namespaces from --all-refs by default; --include-pull-refs opts back in (e.g. for a faithful archival mirror). The run reports how many such refs it dropped (status line + Result.SkippedPullRefs) so an --all-refs conversion never silently omits them.**

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>  
Entire-Checkpoint: 40a74bd2030d+238/-15