This repository was archived by the owner on Aug 25, 2026. It is now read-only.
fix: prevent firstmate pr targeting upstream - #31
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Ship a narrow Firstmate-owned guard so no-mistakes and PR creation for this repository cannot accidentally open or update PRs against kunchenguid/firstmate. The intended target is the captain fork JTInventory/firstmate. The change should use the concrete evidence that intended PR #30 was on JTInventory/firstmate and duplicate upstream PR kunchenguid#171 was opened on kunchenguid/firstmate, keep behavior narrow, avoid unrelated no-mistakes CI monitor, durable-profile, behavior-test runtime, watcher, secondmate routing, or fm-send changes, add focused fork-parent target tests, document only the captain fork target, and stop before PR creation if any path would target kunchenguid/firstmate.
What Changed
JTInventory/firstmateand fails closed for upstreamkunchenguid/firstmate, targetless local gates, unsafeno-mistakesremotes, and override attempts.Risk Assessment
✅ Low: Captain, the change is narrow, fail-closed, and covered by focused behavior tests for the target-bypass cases reviewed here.
Testing
The prompt-provided baseline had already passed; I then exercised the focused fork-parent guard suite, captured manual operator-facing accept/block transcripts, verified the live worktree target, and reran the full configured command with the new guard prefix. All required checks passed, with intentional upstream-target cases exiting nonzero and printing the expected block messages.
Evidence: manual PR target guard transcript
Evidence: focused PR target guard test output
Evidence: live worktree guard stdout
ok: PR target repo jtinventory/firstmate verifiedPipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (4) ✅
bin/fm-no-mistakes-pr-target-guard.sh:64- The guard only inspects the first origin push URL. Git supports multiple push URLs, andgit remote get-url --push originreturns one unless--allis used, so a repo with an allowed first push URL plus a secondhttps://github.com/kunchenguid/firstmatepush URL would pass whilegit push origincan still target the upstream repo. Iterate over all origin fetch/push URLs, and use--get-allfor gate config checks too.🔧 Fix: Check every PR target URL
1 warning still open:
bin/fm-no-mistakes-pr-target-guard.sh:78- The guard silently skips ano-mistakesremote when that remote URL is not a local directory. Iforiginis correctly set toJTInventory/firstmatebutgit push no-mistakespoints directly athttps://github.com/kunchenguid/firstmate, this check continues instead of blocking, so the new fail-closed guard still has a PR-target bypass. Treat a non-localno-mistakesURL as a target URL to verify, or fail closed when it cannot be inspected as a local gate.🔧 Fix: Captain, verify no-mistakes remote targets
1 warning still open:
bin/fm-no-mistakes-pr-target-guard.sh:78- When theno-mistakesremote points at an existing directory, the guard treats it as inspected even ifgit --git-dir="$gate" configfails or returns noremote.origin.url/remote.origin.pushurl. Because those reads are swallowed with|| true, an uninspectable or targetless local gate can still pass asok; fail closed unless the local gate yields at least one verifiable target URL.🔧 Fix: Fail closed on targetless local gates
2 warnings still open:
bin/fm-no-mistakes-pr-target-guard.sh:83- The guard checksremote.no-mistakes.urlbut never checksremote.no-mistakes.pushurl. Git usespushurlforgit push no-mistakes, so a repo with a safe no-mistakes fetch URL plusremote.no-mistakes.pushurl=https://github.com/kunchenguid/firstmatewould pass this guard and still push through the upstream target. Inspect everygit remote get-url --push --all no-mistakesURL too.bin/fm-no-mistakes-pr-target-guard.sh:12- The expected target can be changed with either an argument orFM_FIRSTMATE_PR_TARGET_REPO, which creates an allowlist escape hatch for the exact upstream repo this guard is meant to block. If the captain-owned target must remain onlyJTInventory/firstmate, remove the override or fail closed when the expected repo normalizes tokunchenguid/firstmate.🔧 Fix: Captain, pin PR target guard
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"Baseline already provided by the prompt:command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"bash tests/fm-no-mistakes-pr-target-guard.test.sh > /tmp/no-mistakes-evidence/01KWEVTCZ3EBK8BCQV84XWNHHK/focused-pr-target-guard-test.log 2>&1bash bin/fm-no-mistakes-pr-target-guard.sh > /tmp/no-mistakes-evidence/01KWEVTCZ3EBK8BCQV84XWNHHK/live-worktree-guard.out 2> /tmp/no-mistakes-evidence/01KWEVTCZ3EBK8BCQV84XWNHHK/live-worktree-guard.errManual CLI evidence cases recorded in/tmp/no-mistakes-evidence/01KWEVTCZ3EBK8BCQV84XWNHHK/manual-pr-target-guard-transcript.txt: safe captain fork, blocked upstream origin, blocked upstream no-mistakes gate, blocked parent override, live worktree guard.command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; bash bin/fm-no-mistakes-pr-target-guard.sh || exit 1; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.