From f0ee218af63b9c503c3c9dcbc2a17a13529e683b Mon Sep 17 00:00:00 2001 From: Rome-1 Date: Thu, 1 Oct 2026 17:09:41 -0700 Subject: [PATCH] fix(run): refuse an auto-detected branch that is not on the remote `rafter run` scans the remote repository, but when it auto-detected the current local branch it never checked that the branch had been pushed. The scan was queued and then failed on the backend minutes later. When both repo and branch come from the local checkout, ask origin with `git ls-remote --exit-code --heads`. If the remote answers without the branch, exit 1 with a message to push it or pass --branch. If origin cannot be reached, proceed as before. If the pushed commit differs from local HEAD, note that the scan covers the pushed commit. Explicit --branch and CI-provided branches are unchanged. Same behavior in the Node and Python CLIs, each with a test against a real local bare remote. --- node/src/commands/backend/run.ts | 27 ++++++++++++++-- node/src/utils/git.ts | 34 +++++++++++++++++++-- node/tests/remote-branch.test.ts | 36 ++++++++++++++++++++++ python/rafter_cli/commands/backend.py | 26 +++++++++++++++- python/rafter_cli/utils/git.py | 44 ++++++++++++++++++++++++--- python/tests/test_git_utils.py | 31 +++++++++++++++++++ python/tests/test_scan_remote.py | 6 ++++ shared-docs/CLI_SPEC.md | 2 +- 8 files changed, 195 insertions(+), 11 deletions(-) create mode 100644 node/tests/remote-branch.test.ts diff --git a/node/src/commands/backend/run.ts b/node/src/commands/backend/run.ts index db46df9f..e4f15c61 100644 --- a/node/src/commands/backend/run.ts +++ b/node/src/commands/backend/run.ts @@ -1,6 +1,6 @@ import { Command } from "commander"; import ora from "ora"; -import { detectRepo } from "../../utils/git.js"; +import { detectRepo, git, remoteBranchSha } from "../../utils/git.js"; import { API, resolveKey, @@ -96,8 +96,9 @@ export async function runRemoteScan(opts: RunOpts): Promise { const ghToken = opts.githubToken || process.env.RAFTER_GITHUB_TOKEN; let repo: string | undefined, branch: string | undefined; let detectedProvider: string | undefined, detectedRepoUrl: string | undefined; + let localBranch: boolean | undefined; try { - ({ repo, branch, provider: detectedProvider, repo_url: detectedRepoUrl } = detectRepo({ + ({ repo, branch, provider: detectedProvider, repo_url: detectedRepoUrl, local_branch: localBranch } = detectRepo({ repo: opts.repo, branch: opts.branch, quiet: opts.quiet, @@ -111,6 +112,28 @@ export async function runRemoteScan(opts: RunOpts): Promise { process.exit(EXIT_GENERAL_ERROR); } + // The backend clones the remote, so an auto-detected branch that was never + // pushed can only fail there. Say so now instead of queueing that scan. + if (localBranch) { + const remoteSha = remoteBranchSha(branch!); + if (remoteSha === null) { + console.error( + `Branch "${branch}" does not exist on the remote (origin). Rafter scans the remote ` + + "repository: push the branch first, or pass --branch to scan one that exists." + ); + process.exit(EXIT_GENERAL_ERROR); + } + if (remoteSha && !opts.quiet) { + let head: string | undefined; + try { head = git("rev-parse HEAD"); } catch { head = undefined; } + if (head && head !== remoteSha) { + console.error( + `Note: local HEAD differs from origin/${branch}; the scan covers the pushed commit ${remoteSha.slice(0, 7)}.` + ); + } + } + } + // Explicit flags override inferred values. const provider = opts.provider ?? detectedProvider; const repoUrl = opts.repoUrl ?? detectedRepoUrl; diff --git a/node/src/utils/git.ts b/node/src/utils/git.ts index 38618753..d8446ffe 100644 --- a/node/src/utils/git.ts +++ b/node/src/utils/git.ts @@ -1,4 +1,4 @@ -import { execSync } from "child_process"; +import { execFileSync, execSync } from "child_process"; export function git(cmd: string): string { return execSync(`git ${cmd}`, { stdio: ["ignore", "pipe", "ignore"] }) @@ -6,6 +6,32 @@ export function git(cmd: string): string { .trim(); } +/** + * Look up `branch` on the `origin` remote. + * + * Returns the remote commit SHA, `null` when the remote answered and has no + * such branch, or `undefined` when it could not be asked (offline, auth + * failure, timeout). Callers treat `undefined` as unknown and carry on. + */ +export function remoteBranchSha(branch: string, cwd?: string): string | null | undefined { + try { + const out = execFileSync( + "git", + ["ls-remote", "--exit-code", "--heads", "origin", `refs/heads/${branch}`], + { + cwd, + stdio: ["ignore", "pipe", "ignore"], + timeout: 15_000, + env: { ...process.env, GIT_TERMINAL_PROMPT: "0" }, + } + ).toString(); + return out.split(/\s+/)[0] || undefined; + } catch (e: any) { + // --exit-code: status 2 means the remote has no matching ref. + return e?.status === 2 ? null : undefined; + } +} + /** * Return the current branch name. * @@ -155,6 +181,8 @@ export interface DetectedRepo { branch?: string; provider?: Provider; repo_url?: string; + /** Both repo and branch came from the local checkout (origin + HEAD). */ + local_branch?: boolean; } const AUTO_DETECT_FAILURE = @@ -191,6 +219,7 @@ export function detectRepo(opts: { repo?: string; branch?: string; quiet?: boole // A rejection from parseRemote (unrecognized host) is deliberately NOT // swallowed into the generic message below — it names the offending // remote, which is the actionable part. + const repoFromOrigin = !repoSlug; if (!repoSlug) { let remoteUrl: string; try { @@ -204,6 +233,7 @@ export function detectRepo(opts: { repo?: string; branch?: string; quiet?: boole repoUrl = inferred.repoUrl; } + const localBranch = repoFromOrigin && !branch; if (!branch) { branch = safeBranch(git); } @@ -211,5 +241,5 @@ export function detectRepo(opts: { repo?: string; branch?: string; quiet?: boole if ((!opts.repo || !opts.branch) && !opts.quiet) { console.error(`Repo auto-detected: ${repoSlug} @ ${branch} (note: scanning remote)`); } - return { repo: repoSlug, branch, provider, repo_url: repoUrl }; + return { repo: repoSlug, branch, provider, repo_url: repoUrl, local_branch: localBranch }; } diff --git a/node/tests/remote-branch.test.ts b/node/tests/remote-branch.test.ts new file mode 100644 index 00000000..c9fea127 --- /dev/null +++ b/node/tests/remote-branch.test.ts @@ -0,0 +1,36 @@ +import { describe, it, expect } from "vitest"; +import { execFileSync } from "child_process"; +import fs from "fs"; +import os from "os"; +import path from "path"; +import { remoteBranchSha } from "../src/utils/git.js"; + +// `rafter run` scans the remote, so it must tell an unpushed local branch +// (remote answers, branch absent) apart from a pushed one and from a remote +// it cannot reach. Real git against a local bare origin, no network. +describe("remoteBranchSha", () => { + it("returns the SHA for a pushed branch, null for an unpushed one, undefined when origin is unreachable", () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "rafter-remote-branch-")); + try { + const g = (cwd: string, ...args: string[]) => + execFileSync("git", args, { cwd, stdio: ["ignore", "pipe", "ignore"] }).toString().trim(); + const bare = path.join(root, "origin.git"); + const work = path.join(root, "work"); + fs.mkdirSync(work); + g(root, "init", "-q", "--bare", bare); + g(work, "init", "-q", "-b", "main"); + g(work, "-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", "--allow-empty", "-m", "init"); + g(work, "remote", "add", "origin", bare); + g(work, "push", "-q", "origin", "main"); + g(work, "checkout", "-q", "-b", "task/unpushed"); + + expect(remoteBranchSha("main", work)).toBe(g(work, "rev-parse", "main")); + expect(remoteBranchSha("task/unpushed", work)).toBeNull(); + + g(work, "remote", "set-url", "origin", path.join(root, "missing.git")); + expect(remoteBranchSha("main", work)).toBeUndefined(); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); +}); diff --git a/python/rafter_cli/commands/backend.py b/python/rafter_cli/commands/backend.py index 90ead2f4..198dd8a7 100644 --- a/python/rafter_cli/commands/backend.py +++ b/python/rafter_cli/commands/backend.py @@ -24,7 +24,7 @@ resolve_key, write_payload, ) -from ..utils.git import detect_repo +from ..utils.git import _run, branch_from_env, detect_repo, remote_branch_sha def _plus_approval_gate_enabled() -> bool: @@ -408,6 +408,30 @@ def _do_remote_scan( if not (repo and branch) and not quiet: print(f"Repo auto-detected: {repo_slug} @ {branch_name} (note: scanning remote)", file=sys.stderr) + # The backend clones the remote, so an auto-detected branch that was never + # pushed can only fail there. Say so now instead of queueing that scan. + # A repo_url is inferred only when the slug came from the origin remote. + if detected_repo_url and not branch and not branch_from_env(): + remote_sha = remote_branch_sha(branch_name) + if remote_sha is None: + print( + f'Branch "{branch_name}" does not exist on the remote (origin). Rafter scans ' + "the remote repository: push the branch first, or pass --branch to scan one that exists.", + file=sys.stderr, + ) + raise typer.Exit(code=EXIT_GENERAL_ERROR) + if remote_sha and not quiet: + try: + head = _run(["git", "rev-parse", "HEAD"]) + except Exception: + head = None + if head and head != remote_sha: + print( + f"Note: local HEAD differs from origin/{branch_name}; " + f"the scan covers the pushed commit {remote_sha[:7]}.", + file=sys.stderr, + ) + # Explicit flags override inferred values. resolved_provider = provider or detected_provider resolved_repo_url = repo_url or detected_repo_url diff --git a/python/rafter_cli/utils/git.py b/python/rafter_cli/utils/git.py index 79acd4cb..f1b91e1e 100644 --- a/python/rafter_cli/utils/git.py +++ b/python/rafter_cli/utils/git.py @@ -27,6 +27,44 @@ def is_inside_repo() -> bool: return False +def remote_branch_sha(branch: str, cwd: str | None = None) -> str | None | bool: + """Look up ``branch`` on the ``origin`` remote. + + Returns the remote commit SHA, ``None`` when the remote answered and has + no such branch, or ``False`` when it could not be asked (offline, auth + failure, timeout). Callers treat ``False`` as unknown and carry on. + """ + import os + + try: + out = subprocess.run( + ["git", "ls-remote", "--exit-code", "--heads", "origin", f"refs/heads/{branch}"], + cwd=cwd, + capture_output=True, + text=True, + timeout=15, + env={**os.environ, "GIT_TERMINAL_PROMPT": "0"}, + ) + except (OSError, subprocess.TimeoutExpired): + return False + if out.returncode == 2: # --exit-code: no matching ref on the remote + return None + if out.returncode != 0: + return False + return out.stdout.split()[0] if out.stdout.split() else False + + +def branch_from_env() -> str | None: + """The branch a CI provider names in its environment, if any.""" + import os + + return ( + os.getenv("GITHUB_REF_NAME") + or os.getenv("CI_COMMIT_BRANCH") + or os.getenv("CI_BRANCH") + ) + + def safe_branch() -> str: """Return the current branch name. @@ -178,11 +216,7 @@ def detect_repo( import os repo_env = os.getenv("GITHUB_REPOSITORY") or os.getenv("CI_REPOSITORY") - branch_env = ( - os.getenv("GITHUB_REF_NAME") - or os.getenv("CI_COMMIT_BRANCH") - or os.getenv("CI_BRANCH") - ) + branch_env = branch_from_env() repo_slug = repo or repo_env branch_name = branch or branch_env provider: str | None = None diff --git a/python/tests/test_git_utils.py b/python/tests/test_git_utils.py index 23f2662c..52ba0734 100644 --- a/python/tests/test_git_utils.py +++ b/python/tests/test_git_utils.py @@ -14,6 +14,7 @@ get_git_root, provider_for_host, infer_remote, + remote_branch_sha, ) @@ -385,3 +386,33 @@ def test_raises_when_not_in_repo_and_no_env(self, monkeypatch): with patch("rafter_cli.utils.git.is_inside_repo", return_value=False): with pytest.raises(RuntimeError, match="Could not auto-detect"): detect_repo() + + +# ── remote_branch_sha (real git, local bare origin) ───────────────── + + +def test_remote_branch_sha_tells_unpushed_from_pushed_and_unreachable(tmp_path): + """`rafter run` scans the remote, so it must tell an unpushed local branch + (remote answers, branch absent) apart from a pushed one and from a remote + it cannot reach.""" + + def g(cwd, *args): + return subprocess.check_output( + ["git", *args], cwd=cwd, text=True, stderr=subprocess.DEVNULL + ).strip() + + bare = tmp_path / "origin.git" + work = tmp_path / "work" + work.mkdir() + g(tmp_path, "init", "-q", "--bare", str(bare)) + g(work, "init", "-q", "-b", "main") + g(work, "-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", "--allow-empty", "-m", "init") + g(work, "remote", "add", "origin", str(bare)) + g(work, "push", "-q", "origin", "main") + g(work, "checkout", "-q", "-b", "task/unpushed") + + assert remote_branch_sha("main", cwd=str(work)) == g(work, "rev-parse", "main") + assert remote_branch_sha("task/unpushed", cwd=str(work)) is None + + g(work, "remote", "set-url", "origin", str(tmp_path / "missing.git")) + assert remote_branch_sha("main", cwd=str(work)) is False diff --git a/python/tests/test_scan_remote.py b/python/tests/test_scan_remote.py index 4acf0198..a241552a 100644 --- a/python/tests/test_scan_remote.py +++ b/python/tests/test_scan_remote.py @@ -200,6 +200,8 @@ def test_provider_without_any_repo_url_is_omitted(self, _mock_repo, mock_post): assert "provider" not in body assert "repo_url" not in body + # Auto-detected branch: keep the remote-branch lookup off the network. + @patch("rafter_cli.commands.backend.remote_branch_sha", new=lambda *a, **k: False) @patch("rafter_cli.commands.backend.api_post") @patch( "rafter_cli.commands.backend.detect_repo", @@ -224,6 +226,8 @@ def test_inferred_gitlab_provider_flows_into_body(self, _mock_repo, mock_post): assert body["provider"] == "gitlab" assert body["repo_url"] == "https://gitlab.com/group/project" + # Auto-detected branch: keep the remote-branch lookup off the network. + @patch("rafter_cli.commands.backend.remote_branch_sha", new=lambda *a, **k: False) @patch("rafter_cli.commands.backend.api_post") @patch( "rafter_cli.commands.backend.detect_repo", @@ -305,6 +309,8 @@ def test_prints_scan_id_when_not_quiet(self, _mock_repo, mock_post, capsys): err = capsys.readouterr().err assert "s-xyz" in err + # Auto-detected branch: keep the remote-branch lookup off the network. + @patch("rafter_cli.commands.backend.remote_branch_sha", new=lambda *a, **k: False) @patch("rafter_cli.commands.backend.api_post") @patch("rafter_cli.commands.backend.detect_repo", return_value=("owner/repo", "main", "github", "https://github.com/owner/repo")) def test_auto_detect_message_when_not_explicit(self, _mock_repo, mock_post, capsys): diff --git a/shared-docs/CLI_SPEC.md b/shared-docs/CLI_SPEC.md index 30f59668..3fa3e74e 100644 --- a/shared-docs/CLI_SPEC.md +++ b/shared-docs/CLI_SPEC.md @@ -77,7 +77,7 @@ Trigger a new security scan for a repository. - `-k, --api-key TEXT` — API key. Resolution order: this flag → `RAFTER_API_KEY` env → `backend.apiKey` in global config (see `rafter agent config`) - `-r, --repo TEXT` — org/repo (default: auto-detected from git remote; errors if the remote's host isn't a recognized GitHub/GitLab/Bitbucket/Gitea host — pass this flag explicitly for anything else) -- `-b, --branch TEXT` — branch (default: current branch; errors on a detached HEAD instead of submitting a commit SHA or guessing 'main' — pass this flag explicitly) +- `-b, --branch TEXT` — branch (default: current branch; errors on a detached HEAD instead of submitting a commit SHA or guessing 'main' — pass this flag explicitly). When both repo and branch are auto-detected, rafter asks `origin` whether the branch exists and exits `1` if it does not, since the scan runs against the remote; if `origin` cannot be reached it proceeds. When the pushed commit differs from local `HEAD` it notes that the scan covers the pushed commit - `-f, --format [json|md]` — output format (default: md) - `-m, --mode [fast|plus]` — scan mode (default: fast). Fast runs SAST, secret detection, and dependency checks. Plus adds agentic deep-dive analysis that examines your codebase the way a professional cybersecurity auditor would — tracing data flows and reasoning about business logic on top of the full SAST/SCA toolchain. **Plus is a paid tier that consumes credits.** - `--github-token TEXT` — GitHub PAT for private repos (or `RAFTER_GITHUB_TOKEN` env var)