From 780c0fd4e3e15506e3868c29593754e5d934038d Mon Sep 17 00:00:00 2001 From: jdalton Date: Tue, 4 Aug 2026 11:04:11 -0400 Subject: [PATCH] fix(ci): send real branch and PR for CI scans Backport of the SURF-634 fix from main. In a CI checkout HEAD is detached, so gitBranch fell through to a short commit SHA. A SHA can never match the repo's default branch, so those scans never showed up in the Main or PR tabs of the dashboard. Prefer the branch the workflow says it is on: GITHUB_HEAD_REF for a pull request, else GITHUB_REF_NAME when GITHUB_REF_TYPE is branch, and only then the SHA. socket ci also hardcoded pullRequest to 0, so a scan from a pull_request event could not land in the PR tab either. Derive the number from GITHUB_REF, which is refs/pull//merge on those events. main carries the branch logic in packages/cli/src/util/git/git-remote-info.mts. v1.x is the flat tree, so it lands in src/utils/git.mts as getCiBranch, and the environment is read straight from process.env because GITHUB_HEAD_REF and GITHUB_REF are not part of the constants.ENV snapshot. make_default_branch is untouched and still defaults to false. --- src/commands/ci/handle-ci.mts | 16 +++- src/commands/ci/handle-ci.test.mts | 49 ++++++++++ src/utils/git.mts | 36 ++++++++ src/utils/git.test.mts | 140 +++++++++++++++++++++++++++++ 4 files changed, 240 insertions(+), 1 deletion(-) create mode 100644 src/commands/ci/handle-ci.test.mts create mode 100644 src/utils/git.test.mts diff --git a/src/commands/ci/handle-ci.mts b/src/commands/ci/handle-ci.mts index 776e3561e1..36b3e25f4b 100644 --- a/src/commands/ci/handle-ci.mts +++ b/src/commands/ci/handle-ci.mts @@ -1,4 +1,5 @@ import { debugDir, debugFn } from '@socketsecurity/registry/lib/debug' +import { envAsString } from '@socketsecurity/registry/lib/env' import { logger } from '@socketsecurity/registry/lib/logger' import { getDefaultOrgSlug } from './fetch-default-org-slug.mts' @@ -11,6 +12,19 @@ import { import { serializeResultJson } from '../../utils/serialize-result-json.mts' import { handleCreateNewScan } from '../scan/handle-create-new-scan.mts' +/** + * Derive the pull request number from the CI environment. GitHub Actions + * pull_request events check out `refs/pull//merge`, so the number is + * recoverable from GITHUB_REF; returns 0 outside a PR run (the API omits + * `pull_request` for falsy values). + */ +export function detectCiPullRequestNumber(): number { + const match = /^refs\/pull\/(\d+)\//.exec( + envAsString(process.env['GITHUB_REF']), + ) + return match ? Number(match[1]) : 0 +} + export async function handleCi(autoManifest: boolean): Promise { debugFn('notice', 'Starting CI scan') debugDir('inspect', { autoManifest }) @@ -49,7 +63,7 @@ export async function handleCi(autoManifest: boolean): Promise { outputKind: 'json', // When 'pendingHead' is true, it requires 'branchName' set and 'tmp' false. pendingHead: true, - pullRequest: 0, + pullRequest: detectCiPullRequestNumber(), reach: { dynamicSbomInference: false, excludePaths: [], diff --git a/src/commands/ci/handle-ci.test.mts b/src/commands/ci/handle-ci.test.mts new file mode 100644 index 0000000000..9688e1c2e1 --- /dev/null +++ b/src/commands/ci/handle-ci.test.mts @@ -0,0 +1,49 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { detectCiPullRequestNumber } from './handle-ci.mts' + +let originalGithubRef: string | undefined + +describe('detectCiPullRequestNumber', () => { + beforeEach(() => { + originalGithubRef = process.env['GITHUB_REF'] + delete process.env['GITHUB_REF'] + }) + + afterEach(() => { + if (originalGithubRef === undefined) { + delete process.env['GITHUB_REF'] + } else { + process.env['GITHUB_REF'] = originalGithubRef + } + }) + + it('derives the number from a pull request merge ref', () => { + process.env['GITHUB_REF'] = 'refs/pull/482/merge' + expect(detectCiPullRequestNumber()).toBe(482) + }) + + it('derives the number from a pull request head ref', () => { + process.env['GITHUB_REF'] = 'refs/pull/482/head' + expect(detectCiPullRequestNumber()).toBe(482) + }) + + it('returns 0 for a branch push', () => { + process.env['GITHUB_REF'] = 'refs/heads/feature-branch' + expect(detectCiPullRequestNumber()).toBe(0) + }) + + it('returns 0 for a tag push', () => { + process.env['GITHUB_REF'] = 'refs/tags/v1.2.3' + expect(detectCiPullRequestNumber()).toBe(0) + }) + + it('returns 0 when GITHUB_REF is not a numbered pull ref', () => { + process.env['GITHUB_REF'] = 'refs/pull/not-a-number/merge' + expect(detectCiPullRequestNumber()).toBe(0) + }) + + it('returns 0 outside GitHub Actions', () => { + expect(detectCiPullRequestNumber()).toBe(0) + }) +}) diff --git a/src/utils/git.mts b/src/utils/git.mts index c205e17165..eda0efd265 100644 --- a/src/utils/git.mts +++ b/src/utils/git.mts @@ -21,11 +21,13 @@ * Repository Information: * - detectDefaultBranch: Find default branch (main/master/develop/etc) * - getBaseBranch: Determine base branch (respects GitHub Actions env) + * - getCiBranch: Branch name a GitHub Actions run is on * - getRepoInfo: Extract owner/repo from git remote URL * - gitBranch: Get current branch or commit hash */ import { debugDir, debugFn, isDebug } from '@socketsecurity/registry/lib/debug' +import { envAsString } from '@socketsecurity/registry/lib/env' import { normalizePath } from '@socketsecurity/registry/lib/path' import { isSpawnError, spawn } from '@socketsecurity/registry/lib/spawn' @@ -77,6 +79,32 @@ export async function getBaseBranch(cwd = process.cwd()): Promise { return 'main' } +/** + * The branch a GitHub Actions workflow run is on, or undefined when the + * environment does not identify one. Read straight from process.env because + * GITHUB_HEAD_REF is not part of the constants.ENV snapshot. + */ +export function getCiBranch(): string | undefined { + // The head branch of a pull request, only set for pull_request and + // pull_request_target events. Checked first because GITHUB_REF_NAME is + // '/merge' on those events, which is not a branch name. + // https://docs.github.com/en/actions/reference/workflows-and-actions/variables#default-environment-variables + const githubHeadRef = envAsString(process.env['GITHUB_HEAD_REF']) + if (githubHeadRef) { + return githubHeadRef + } + // The pushed ref. GITHUB_REF_TYPE tells branches and tags apart, and a tag + // is not a branch name. + const githubRefName = envAsString(process.env['GITHUB_REF_NAME']) + if ( + envAsString(process.env['GITHUB_REF_TYPE']) === 'branch' && + githubRefName + ) { + return githubRefName + } + return undefined +} + export type RepoInfo = { owner: string repo: string @@ -133,6 +161,14 @@ export async function gitBranch( // Expected in detached HEAD state, fallback to rev-parse. debugDir('inspect', { message: 'In detached HEAD state', error: e }) } + // Detached HEAD is the normal state in CI checkouts (actions/checkout), + // where the commit-SHA fallback below would mislabel the scan's branch — a + // SHA can never match the repo's default branch, so scans vanish from the + // Main/PR tabs. Prefer the branch the CI run says it is on. + const ciBranch = getCiBranch() + if (ciBranch) { + return ciBranch + } // Fallback to using rev-parse to get the short commit hash in a // detached HEAD state. try { diff --git a/src/utils/git.test.mts b/src/utils/git.test.mts new file mode 100644 index 0000000000..8294750232 --- /dev/null +++ b/src/utils/git.test.mts @@ -0,0 +1,140 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' + +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { spawn } from '@socketsecurity/registry/lib/spawn' + +import { getCiBranch, gitBranch } from './git.mts' + +// GitHub Actions sets these in its own runs, so they have to be cleared for +// the tests to exercise anything other than the CI job they run inside. +const GITHUB_ENV_VARS = [ + 'GITHUB_HEAD_REF', + 'GITHUB_REF_NAME', + 'GITHUB_REF_TYPE', +] + +const originalEnv = new Map() + +async function createTempRepo(): Promise { + const repoPath = mkdtempSync(path.join(tmpdir(), 'socket-git-branch-')) + const options = { cwd: repoPath } + await spawn('git', ['init', '--initial-branch', 'feature-branch'], options) + await spawn('git', ['config', 'user.email', 'test@socket.dev'], options) + await spawn('git', ['config', 'user.name', 'Socket Test'], options) + await spawn('git', ['config', 'commit.gpgsign', 'false'], options) + writeFileSync(path.join(repoPath, 'README.md'), '# test\n') + await spawn('git', ['add', 'README.md'], options) + await spawn('git', ['commit', '-m', 'Initial commit'], options) + return repoPath +} + +describe('getCiBranch', () => { + beforeEach(() => { + for (const name of GITHUB_ENV_VARS) { + originalEnv.set(name, process.env[name]) + delete process.env[name] + } + }) + + afterEach(() => { + for (const name of GITHUB_ENV_VARS) { + const value = originalEnv.get(name) + if (value === undefined) { + delete process.env[name] + } else { + process.env[name] = value + } + } + originalEnv.clear() + }) + + it('returns the pull request head branch', () => { + process.env['GITHUB_HEAD_REF'] = 'feature/pr-branch' + expect(getCiBranch()).toBe('feature/pr-branch') + }) + + it('prefers the pull request head branch over the merge ref', () => { + process.env['GITHUB_HEAD_REF'] = 'feature/pr-branch' + // What GitHub Actions actually sets on a pull_request event. + process.env['GITHUB_REF_NAME'] = '123/merge' + process.env['GITHUB_REF_TYPE'] = 'branch' + expect(getCiBranch()).toBe('feature/pr-branch') + }) + + it('returns the pushed branch ref outside a pull request', () => { + process.env['GITHUB_REF_NAME'] = 'main' + process.env['GITHUB_REF_TYPE'] = 'branch' + expect(getCiBranch()).toBe('main') + }) + + it('ignores a tag ref', () => { + process.env['GITHUB_REF_NAME'] = 'v1.2.3' + process.env['GITHUB_REF_TYPE'] = 'tag' + expect(getCiBranch()).toBeUndefined() + }) + + it('returns undefined outside GitHub Actions', () => { + expect(getCiBranch()).toBeUndefined() + }) +}) + +describe('gitBranch', () => { + let repoPath = '' + + beforeEach(async () => { + for (const name of GITHUB_ENV_VARS) { + originalEnv.set(name, process.env[name]) + delete process.env[name] + } + repoPath = await createTempRepo() + }) + + afterEach(() => { + for (const name of GITHUB_ENV_VARS) { + const value = originalEnv.get(name) + if (value === undefined) { + delete process.env[name] + } else { + process.env[name] = value + } + } + originalEnv.clear() + rmSync(repoPath, { force: true, recursive: true }) + }) + + it('returns the checked out branch', async () => { + expect(await gitBranch(repoPath)).toBe('feature-branch') + }) + + it('falls back to the commit hash in a detached HEAD with no CI env', async () => { + await spawn('git', ['checkout', '--detach'], { cwd: repoPath }) + const shortHash = ( + await spawn('git', ['rev-parse', '--short', 'HEAD'], { cwd: repoPath }) + ).stdout + expect(await gitBranch(repoPath)).toBe(shortHash) + }) + + it('returns the pull request head branch in a detached HEAD', async () => { + await spawn('git', ['checkout', '--detach'], { cwd: repoPath }) + process.env['GITHUB_HEAD_REF'] = 'feature/pr-branch' + process.env['GITHUB_REF_NAME'] = '123/merge' + process.env['GITHUB_REF_TYPE'] = 'branch' + expect(await gitBranch(repoPath)).toBe('feature/pr-branch') + }) + + it('returns the pushed branch ref in a detached HEAD', async () => { + await spawn('git', ['checkout', '--detach'], { cwd: repoPath }) + process.env['GITHUB_REF_NAME'] = 'main' + process.env['GITHUB_REF_TYPE'] = 'branch' + expect(await gitBranch(repoPath)).toBe('main') + }) + + it('prefers the checked out branch over the CI env', async () => { + process.env['GITHUB_REF_NAME'] = 'main' + process.env['GITHUB_REF_TYPE'] = 'branch' + expect(await gitBranch(repoPath)).toBe('feature-branch') + }) +})