diff --git a/.changeset/sdk-7009-ci-branch-detection.md b/.changeset/sdk-7009-ci-branch-detection.md new file mode 100644 index 0000000..e75d912 --- /dev/null +++ b/.changeset/sdk-7009-ci-branch-detection.md @@ -0,0 +1,5 @@ +--- +"@wdio/browserstack-service": patch +--- + +Detect the git branch in detached-HEAD CI checkouts. CI systems typically check out a commit SHA directly (`git checkout `), leaving the repo in a detached-HEAD state where `git-repo-info` cannot resolve a branch — so builds reported no branch and dropped out of branch-based dashboards and filters. `getGitMetaData` now resolves the branch in this precedence: an explicit `BROWSERSTACK_GIT_BRANCH` override (which wins even over a git-repo-info-resolved branch, so it can correct a mis-detected one) → `git-repo-info` → the CI provider's branch env var → a `git for-each-ref --points-at HEAD` backstop. Provider-specific env vars are trusted directly; generic names (`BRANCH`, `GIT_BRANCH`, …) are only used when a CI environment is detected. (SDK-7009) diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index 136eebd..ee1dfe0 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -981,6 +981,134 @@ export function getCiInfo () { return null } +// Reject anything a git ref can't legally be OR that is unsafe to propagate: +// whitespace/control chars, the git-forbidden set (~ ^ : ? * [ \), and `..`. +// This is a denylist (git ref names are otherwise permissive — `#`, `+`, `/`, +// `-`, `_`, `.` are all valid), so a legal branch name is never dropped. +// eslint-disable-next-line no-control-regex -- control chars (\x00-\x1f) are exactly what we reject +const UNSAFE_BRANCH_PATTERN = /[\x00-\x20~^:?*[\\]/ + +// CI-specific, namespaced branch env vars in priority order. These are unique +// to their CI provider, so reading them unconditionally is safe. Values from a +// recognised CI's own variable are trusted even when getCiInfo() can't classify +// the run (some providers set the branch var but not the marker getCiInfo keys on). +const CI_SPECIFIC_BRANCH_ENV = [ + 'GITHUB_HEAD_REF', // GitHub Actions (PR — source branch) + 'GITHUB_REF_NAME', // GitHub Actions (push) + 'CI_COMMIT_BRANCH', // GitLab (branch pipelines only — empty on tag pipelines) + 'CI_COMMIT_REF_NAME', // GitLab (also set to the tag name on tag pipelines) + 'CIRCLE_BRANCH', // CircleCI + 'TRAVIS_PULL_REQUEST_BRANCH', // Travis (PR) + 'TRAVIS_BRANCH', // Travis + 'BITBUCKET_BRANCH', // Bitbucket + 'DRONE_COMMIT_BRANCH', // Drone + 'DRONE_BRANCH', // Drone + 'SEMAPHORE_GIT_BRANCH', // Semaphore + 'BUILDKITE_BRANCH', // Buildkite + 'BUILD_SOURCEBRANCHNAME', // Azure DevOps / VSTS (clean name) + 'BUILD_SOURCEBRANCH', // Azure DevOps / VSTS (refs/heads/...) + 'APPVEYOR_REPO_BRANCH', // Appveyor + 'CODEBUILD_WEBHOOK_HEAD_REF', // AWS CodeBuild + 'bamboo_planRepository_branchName', // Bamboo + 'bamboo_repository_branch_name', // Bamboo + 'WERCKER_GIT_BRANCH', // Wercker + 'VERCEL_GIT_COMMIT_REF', // Vercel + 'CF_BRANCH', // CodeFresh +] + +// Generic, non-namespaced branch env vars. These collide with unrelated +// variables a developer might have exported locally, so they are only trusted +// when getCiInfo() confirms we are actually inside a recognised CI run. +const CI_GENERIC_BRANCH_ENV = [ + 'GIT_LOCAL_BRANCH', // Jenkins git plugin (clean) + 'BRANCH_NAME', // Jenkins multibranch + 'GIT_BRANCH', // Jenkins (often origin/) + 'BRANCH', // Netlify / generic +] + +// Strip a leading refs/heads|tags/ or origin/ prefix (Jenkins GIT_BRANCH is +// `origin/`), reject HEAD and anything that isn't a legal/safe ref. +function normalizeBranchName (raw?: string | null): string | undefined { + if (!raw) { + return undefined + } + const branch = raw.trim() + .replace(/^refs\/(heads|tags)\//, '') + .replace(/^origin\//, '') + if (!branch || branch.length > 256 || branch === 'HEAD' || branch.includes('..') || UNSAFE_BRANCH_PATTERN.test(branch)) { + return undefined + } + return branch +} + +function getBranchFromCIEnv (): string | undefined { + for (const key of CI_SPECIFIC_BRANCH_ENV) { + const val = normalizeBranchName(process.env[key]) + if (val) { + return val + } + } + // Generic names are only trusted inside a recognised CI — otherwise a stray + // local `BRANCH`/`GIT_BRANCH` export would be misreported as the git branch. + if (getCiInfo() !== null) { + for (const key of CI_GENERIC_BRANCH_ENV) { + const val = normalizeBranchName(process.env[key]) + if (val) { + return val + } + } + } + return undefined +} + +// Async (non-blocking) — a synchronous subprocess on getGitMetaData's hot path +// would stall the event loop. execFile takes an argv array (no shell), so the +// ref args cannot be interpreted by a shell. child_process is imported lazily +// (inside the try) so module load never touches it — keeps every test that +// partial-mocks node:child_process working, and degrades to [] if it's absent. +async function gitForEachRef (args: string[]): Promise { + try { + const { execFile } = await import('node:child_process') + // No explicit cwd → runs in process.cwd(), the actual checkout the SDK + // is instrumenting. (git-repo-info's `.root` can point at the main repo + // dir under a git worktree, which would resolve the wrong branch.) + const { stdout } = await promisify(execFile)('git', ['for-each-ref', '--points-at', 'HEAD', ...args], { + encoding: 'utf-8', timeout: 3000 + }) + return stdout.split('\n').map(s => s.trim()).filter(Boolean) + } catch { + // best-effort — degrade silently (git missing, not a repo, timeout, …) + return [] + } +} + +const pickBranch = (lines: string[]): string | undefined => { + for (const line of lines) { + if (line === 'HEAD') { + continue // refs/remotes//HEAD → skip the symbolic pointer + } + const branch = normalizeBranchName(line) + if (branch) { + return branch + } + } + return undefined +} + +// Best-effort CI-agnostic fallback: the branch whose tip is the checked-out +// commit. Resolves detached-HEAD checkouts where the commit is a branch tip +// (the common merge-to-master → CI-builds-master case). Never throws. +// Local heads are queried (and returned) before remotes; each format strips the +// ref prefix via lstrip so the value is a bare branch name for ANY remote, not +// just `origin`. Lazy — the remotes query only runs if no local head matched. +async function getBranchFromGit (): Promise { + const fromHeads = pickBranch(await gitForEachRef(['--format=%(refname:lstrip=2)', 'refs/heads'])) + if (fromHeads) { + return fromHeads + } + return pickBranch(await gitForEachRef(['--format=%(refname:lstrip=3)', 'refs/remotes'])) +} + export async function getGitMetaData () { const info: GitRepoInfo = gitRepoInfo() if (!info.commonGitDir) { @@ -989,11 +1117,22 @@ export async function getGitMetaData () { const { remote } = await pGitconfig(info.commonGitDir) const remotes = remote ? Object.keys(remote).map(remoteName => ({ name: remoteName, url: remote[remoteName].url })) : [] + // git-repo-info reads branch from .git/HEAD; on a detached HEAD (the default + // for most CI checkouts, e.g. `git checkout `) it returns no branch. + // Precedence: explicit BROWSERSTACK_GIT_BRANCH override (wins even over a + // git-repo-info-resolved branch, so it can correct a mis-resolved one) → + // git-repo-info → CI branch env vars → branch whose tip is HEAD. Keeps builds + // in branch-based dashboards/filters. Ref: SDK-7009. + let branch = normalizeBranchName(process.env.BROWSERSTACK_GIT_BRANCH) || info.branch + if (!branch) { + branch = getBranchFromCIEnv() ?? (await getBranchFromGit()) ?? info.branch + } + let gitMetaData : GitMetaData = { name: 'git', sha: info.sha, short_sha: info.abbreviatedSha, - branch: info.branch, + branch: branch, tag: info.tag, committer: info.committer, committer_date: info.committerDate, diff --git a/packages/browserstack-service/tests/util.test.ts b/packages/browserstack-service/tests/util.test.ts index 1260156..0d2d2f0 100644 --- a/packages/browserstack-service/tests/util.test.ts +++ b/packages/browserstack-service/tests/util.test.ts @@ -3,6 +3,7 @@ import type { LaunchResponse } from '../src/types.js' import { describe, expect, it, vi, beforeEach, afterEach, beforeAll, afterAll } from 'vitest' import gitRepoInfo from 'git-repo-info' +import { execFile } from 'node:child_process' import CrashReporter from '../src/crash-reporter.js' import logger from '@wdio/logger' import * as utils from '../src/util.js' @@ -68,6 +69,17 @@ const log = logger('test') vi.mock('fetch') vi.mock('git-repo-info') +// getGitMetaData promisifies gitconfiglocal; the repo also mocks `fs`, so the +// real gitconfiglocal cannot read a config file. Resolve it with no remotes. +vi.mock('gitconfiglocal', () => ({ + default: (_dir: string, cb: (err: Error | null, config: unknown) => void) => cb(null, { remote: {} }) +})) +// Mock only execFile (getGitMetaData's async git for-each-ref backstop); keep +// the rest of node:child_process real so unrelated imports are unaffected. +vi.mock('node:child_process', async (importActual) => { + const actual = await importActual>() + return { ...actual, execFile: vi.fn() } +}) // Fake only Date (not `performance`): these tests need a deterministic system // clock but never advance timers. Faking `performance` too makes performance.now() // negative under the 2020 system time, which Node 18's perf_hooks rejects with @@ -958,6 +970,163 @@ describe('getGitMetaData', () => { // } }) + + // SDK-7009: git-repo-info returns no branch on a detached HEAD (the default + // for CI checkouts). getGitMetaData must recover the branch so the build's + // version_control still reports it. All env mutations are restored in + // finally so a failed assertion can't leak state into other tests. + + // Every branch-carrying env var the fallback reads. Cleared before the + // detached-HEAD tests so the ambient CI (e.g. GitHub Actions sets + // GITHUB_REF_NAME) can't pre-empt the code path under test. + const BRANCH_ENV_KEYS = [ + 'BROWSERSTACK_GIT_BRANCH', 'GITHUB_HEAD_REF', 'GITHUB_REF_NAME', 'CI_COMMIT_BRANCH', + 'CI_COMMIT_REF_NAME', 'CIRCLE_BRANCH', 'TRAVIS_PULL_REQUEST_BRANCH', 'TRAVIS_BRANCH', + 'BITBUCKET_BRANCH', 'DRONE_COMMIT_BRANCH', 'DRONE_BRANCH', 'SEMAPHORE_GIT_BRANCH', + 'BUILDKITE_BRANCH', 'BUILD_SOURCEBRANCHNAME', 'BUILD_SOURCEBRANCH', 'APPVEYOR_REPO_BRANCH', + 'CODEBUILD_WEBHOOK_HEAD_REF', 'bamboo_planRepository_branchName', 'bamboo_repository_branch_name', + 'WERCKER_GIT_BRANCH', 'VERCEL_GIT_COMMIT_REF', 'CF_BRANCH', + 'GIT_LOCAL_BRANCH', 'BRANCH_NAME', 'GIT_BRANCH', 'BRANCH', + ] + // Every env var getCiInfo() gates a non-null return on. Cleared so getCiInfo() + // is null unless a test opts a specific CI in — the generic-var gate + // (`getCiInfo() !== null`) is otherwise non-deterministic under ambient CI. + // Kept exhaustive against src/util.ts getCiInfo(); the getCiInfo()-null/non-null + // guard asserts in the tests below fail loudly if this list ever drifts. + const CI_MARKER_KEYS = [ + 'CI', 'CIRCLECI', 'TRAVIS', 'CI_NAME', 'DRONE', 'SEMAPHORE', 'GITLAB_CI', 'BUILDKITE', 'VERCEL', + 'JENKINS_URL', 'JENKINS_HOME', 'BITBUCKET_COMMIT', 'BITBUCKET_BUILD_NUMBER', + 'TF_BUILD', 'TF_BUILD_BUILDNUMBER', 'APPVEYOR', 'AZURE_HTTP_USER_AGENT', + 'CODEBUILD_BUILD_ID', 'CODEBUILD_RESOLVED_SOURCE_VERSION', 'CODEBUILD_SOURCE_VERSION', + 'bamboo_buildNumber', 'WERCKER', 'WERCKER_MAIN_PIPELINE_STARTED', + 'GCP_PROJECT', 'GCLOUD_PROJECT', 'GOOGLE_CLOUD_PROJECT', 'SHIPPABLE', 'NETLIFY', + 'GITHUB_ACTIONS', 'TEAMCITY_VERSION', 'CONCOURSE', 'CONCOURSE_URL', 'CONCOURSE_USERNAME', + 'CONCOURSE_TEAM', 'GO_JOB_NAME', 'CF_BUILD_ID', + ] + const ALL_ENV_KEYS = [...BRANCH_ENV_KEYS, ...CI_MARKER_KEYS] + const withEnv = async (overrides: Record, fn: () => Promise) => { + const saved: Record = {} + for (const k of ALL_ENV_KEYS) { saved[k] = process.env[k]; delete process.env[k] } + for (const [k, v] of Object.entries(overrides)) { + if (v === undefined) { delete process.env[k] } else { process.env[k] = v } + } + try { await fn() } finally { + for (const k of ALL_ENV_KEYS) { + if (saved[k] === undefined) { delete process.env[k] } else { process.env[k] = saved[k] } + } + } + } + // Mock the async git backstop (promisify(execFile) → resolves {stdout,stderr}). + // A per-call queue lets a test feed the heads query then the remotes query. + const mockGitRefs = (...outputs: string[]) => { + let i = 0 + vi.mocked(execFile).mockImplementation(((...args: unknown[]) => { + const cb = args[args.length - 1] as (e: unknown, r: unknown) => void + const out = i < outputs.length ? outputs[i] : '' + i += 1 + cb(null, { stdout: out, stderr: '' }) + return {} as any + }) as any) + } + + // Default: resolve the git backstop with empty output so any test reaching it + // (without calling mockGitRefs) never hangs on the unresolved-callback mock. + beforeEach(() => mockGitRefs()) + + it('uses git-repo-info branch when present (no fallback)', async () => { + await withEnv({}, async () => { + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: 'develop', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('develop') + }) + }) + + it('BROWSERSTACK_GIT_BRANCH overrides even a git-repo-info-resolved branch', async () => { + await withEnv({ BROWSERSTACK_GIT_BRANCH: 'release/1.2' }, async () => { + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: 'wrong-branch', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('release/1.2') + }) + }) + + it('falls back to a CI branch env var on detached HEAD', async () => { + await withEnv({ GITHUB_REF_NAME: 'master' }, async () => { + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('master') + }) + }) + + it('normalizes CI env branch (strips refs/heads/ prefix)', async () => { + await withEnv({ BROWSERSTACK_GIT_BRANCH: 'refs/heads/release/1.2' }, async () => { + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('release/1.2') + }) + }) + + it('falls back to the git for-each-ref backstop when no branch env is set', async () => { + await withEnv({}, async () => { + // No CI branch env → getBranchFromGit runs. Local heads query resolves + // the branch whose tip is HEAD. + mockGitRefs('master\n') + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('master') + expect(vi.mocked(execFile)).toHaveBeenCalledWith('git', expect.arrayContaining(['for-each-ref', '--points-at', 'HEAD']), expect.anything(), expect.anything()) + }) + }) + + it('git backstop falls through to remotes (bare name) when no local head matches', async () => { + await withEnv({}, async () => { + // heads query empty → remotes query returns a lstrip-3 bare name for a + // non-origin remote (must not leak the remote prefix). + mockGitRefs('', 'main\n') + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('main') + }) + }) + + it('leaves branch undefined when detached HEAD has no env and no branch tip', async () => { + await withEnv({}, async () => { + mockGitRefs('', '') + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toBeUndefined() + }) + }) + + it('ignores a generic BRANCH env var when NOT in a recognised CI', async () => { + // A stray local `BRANCH` export must not be misreported as the branch. + await withEnv({ BRANCH: 'stray-local-value' }, async () => { + // Guard: assert the real detector actually sees "no CI" here — if a + // CI marker leaks past CI_MARKER_KEYS this fails loudly instead of + // silently exercising the in-CI branch. + expect(utils.getCiInfo()).toBeNull() + mockGitRefs('', '') // backstop also empty + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toBeUndefined() + }) + }) + + it('uses a generic GIT_BRANCH var only inside a recognised CI (Jenkins), stripping origin/', async () => { + await withEnv({ JENKINS_URL: 'http://jenkins.local', GIT_BRANCH: 'origin/main' }, async () => { + expect(utils.getCiInfo()).not.toBeNull() // guard: detector sees Jenkins + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('main') + }) + }) + + it('prefers a CI-specific env var over a generic one', async () => { + await withEnv({ GITHUB_ACTIONS: 'true', GITHUB_REF_NAME: 'feature/x', BRANCH: 'generic-loser' }, async () => { + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('feature/x') + }) + }) }) describe('getHookType', () => {