From af1bcd938b8847fccf4f439d2f5ae45dc8510a85 Mon Sep 17 00:00:00 2001 From: Ian Jhumel Bautista Date: Thu, 4 Jun 2026 19:03:22 +0800 Subject: [PATCH] fix(surfaces): skip Code Scanning upload + PR comment for remote targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When `target` is a remote URL the scan covers a different repo than the checkout, so SARIF (file paths + commit) and the PR comment are scoped to the wrong repository — uploading would misattribute Code Scanning alerts. Guard both behind isRemoteTarget(). Also drop the unused context.isFork field and shorten the console footer note so it no longer overruns the ASCII box. --- dist/index.js | 29 ++++++++++++++++++++--------- src/context.ts | 10 +--------- src/git.test.ts | 16 +++++++++++++++- src/git.ts | 8 ++++++++ src/main.ts | 19 +++++++++++++++---- src/report/console.ts | 2 +- 6 files changed, 60 insertions(+), 24 deletions(-) diff --git a/dist/index.js b/dist/index.js index 7783b86..d5f6beb 100644 --- a/dist/index.js +++ b/dist/index.js @@ -84831,13 +84831,9 @@ function getRunContext() { const isPullRequest = ctx.eventName === 'pull_request' || ctx.eventName === 'pull_request_target'; // payload.pull_request is loosely typed (index signature); read defensively. const pr = ctx.payload.pull_request; - let isFork = false; let prHeadRef; if (isPullRequest && pr) { prHeadRef = pr.head?.ref; - const headRepo = pr.head?.repo?.full_name; - const baseRepo = pr.base?.repo?.full_name; - isFork = Boolean(headRepo && baseRepo && headRepo !== baseRepo); } return { eventName: ctx.eventName, @@ -84848,7 +84844,6 @@ function getRunContext() { prNumber: pr?.number, prHeadRef, isPullRequest, - isFork, }; } @@ -84909,10 +84904,18 @@ function evaluateGate(g) { // unit-test without the Actions runtime. Replaces the bash branch/label logic // from the v1 composite action (action.yml branch auto-detect + repo label). Object.defineProperty(exports, "__esModule", ({ value: true })); +exports.isRemoteTarget = isRemoteTarget; exports.repoLabel = repoLabel; exports.refToBranch = refToBranch; exports.resolveBranch = resolveBranch; const GH_URL = /^https?:\/\/github\.com\/([^/]+\/[^/]+?)(?:\.git)?\/?$/; +// isRemoteTarget reports whether the scan target is a remote URL rather than the +// local checkout. Caller-repo-scoped surfaces (Code Scanning SARIF upload, the PR +// comment) must be skipped for a remote target — their paths/commit/PR refer to +// THIS repo, not the scanned one, so they'd misattribute results. +function isRemoteTarget(target) { + return /^https?:\/\//i.test(target.trim()); +} // repoLabel resolves the human-facing repo label for the report. A GitHub URL // target wins; otherwise the workflow's owner/repo; otherwise the raw target. function repoLabel(target, ownerRepo) { @@ -85355,8 +85358,12 @@ async function run() { if (inputs.annotations) { (0, annotations_1.emitAnnotations)(result.findings, inputs.maxAnnotations); } + // A remote URL target scans a different repo than this checkout, so the + // caller-repo-scoped surfaces (Code Scanning upload, PR comment) would + // misattribute results to this repo — skip them. + const remoteTarget = (0, git_1.isRemoteTarget)(inputs.target); // Surface 1b: SARIF → Security tab (needs security-events: write). - if (inputs.uploadSarif) { + if (inputs.uploadSarif && !remoteTarget) { const res = await (0, sarif_1.uploadSarif)(inputs.githubToken, ctx, inputs.sarifFile); core.setOutput('sarif-uploaded', String(res.uploaded)); if (res.uploaded) @@ -85365,6 +85372,9 @@ async function run() { core.warning(res.reason); } else { + if (inputs.uploadSarif && remoteTarget) { + core.info('Skipping Code Scanning upload: remote target — SARIF paths/commit do not match this repository.'); + } core.setOutput('sarif-uploaded', 'false'); } // Downloadable artifact (JSON + SARIF). @@ -85372,8 +85382,9 @@ async function run() { const days = inputs.artifactRetentionDays ? parseInt(inputs.artifactRetentionDays, 10) : undefined; await (0, artifact_1.uploadResults)(inputs.artifactName, [inputs.jsonFile, inputs.sarifFile], days); } - // Surface 2: sticky PR comment (needs pull-requests: write; pull_request only). - if (inputs.commentOnPr && ctx.isPullRequest) { + // Surface 2: sticky PR comment (needs pull-requests: write; pull_request only; + // skipped for a remote target — the comment would describe a different repo). + if (inputs.commentOnPr && ctx.isPullRequest && !remoteTarget) { await (0, comment_1.upsertComment)(inputs.githubToken, ctx, md); } // Surface 3: status-check gating — the job status is the check. @@ -85591,7 +85602,7 @@ function buildConsoleLines(d) { L.push(row(`Max severity: ${d.maxSeverity} Native exit: ${d.nativeExit}`)); L.push(rule()); if (d.projected) { - L.push("Projected = estimate from trustabl's own scoring; listed fixes resolved, nothing new. Not a re-scan."); + L.push('Projected = estimate (listed fixes resolved), not a re-scan.'); } return L; } diff --git a/src/context.ts b/src/context.ts index 556a97c..a7b7fb8 100644 --- a/src/context.ts +++ b/src/context.ts @@ -15,7 +15,6 @@ export interface RunContext { prNumber?: number; prHeadRef?: string; isPullRequest: boolean; - isFork: boolean; } export function getRunContext(): RunContext { @@ -23,17 +22,11 @@ export function getRunContext(): RunContext { const isPullRequest = ctx.eventName === 'pull_request' || ctx.eventName === 'pull_request_target'; // payload.pull_request is loosely typed (index signature); read defensively. - const pr = ctx.payload.pull_request as - | { number?: number; head?: any; base?: any } - | undefined; + const pr = ctx.payload.pull_request as { number?: number; head?: any } | undefined; - let isFork = false; let prHeadRef: string | undefined; if (isPullRequest && pr) { prHeadRef = pr.head?.ref; - const headRepo: string | undefined = pr.head?.repo?.full_name; - const baseRepo: string | undefined = pr.base?.repo?.full_name; - isFork = Boolean(headRepo && baseRepo && headRepo !== baseRepo); } return { @@ -45,6 +38,5 @@ export function getRunContext(): RunContext { prNumber: pr?.number, prHeadRef, isPullRequest, - isFork, }; } diff --git a/src/git.test.ts b/src/git.test.ts index 899679a..c1f655a 100644 --- a/src/git.test.ts +++ b/src/git.test.ts @@ -1,4 +1,18 @@ -import { repoLabel, refToBranch, resolveBranch } from './git'; +import { repoLabel, refToBranch, resolveBranch, isRemoteTarget } from './git'; + +describe('isRemoteTarget', () => { + it('detects http(s) URLs (trimmed)', () => { + expect(isRemoteTarget('https://github.com/o/r')).toBe(true); + expect(isRemoteTarget('http://example.com/x')).toBe(true); + expect(isRemoteTarget(' https://github.com/o/r ')).toBe(true); + }); + it('treats local paths and empty as not remote', () => { + expect(isRemoteTarget('.')).toBe(false); + expect(isRemoteTarget('./sub/dir')).toBe(false); + expect(isRemoteTarget('/abs/path')).toBe(false); + expect(isRemoteTarget('')).toBe(false); + }); +}); describe('repoLabel', () => { it('extracts owner/repo from a GitHub URL, stripping .git and trailing slash', () => { diff --git a/src/git.ts b/src/git.ts index 82a117a..db90391 100644 --- a/src/git.ts +++ b/src/git.ts @@ -4,6 +4,14 @@ const GH_URL = /^https?:\/\/github\.com\/([^/]+\/[^/]+?)(?:\.git)?\/?$/; +// isRemoteTarget reports whether the scan target is a remote URL rather than the +// local checkout. Caller-repo-scoped surfaces (Code Scanning SARIF upload, the PR +// comment) must be skipped for a remote target — their paths/commit/PR refer to +// THIS repo, not the scanned one, so they'd misattribute results. +export function isRemoteTarget(target: string): boolean { + return /^https?:\/\//i.test(target.trim()); +} + // repoLabel resolves the human-facing repo label for the report. A GitHub URL // target wins; otherwise the workflow's owner/repo; otherwise the raw target. export function repoLabel(target: string, ownerRepo: string): string { diff --git a/src/main.ts b/src/main.ts index b08e59c..2e515e3 100644 --- a/src/main.ts +++ b/src/main.ts @@ -10,7 +10,7 @@ import { resolveTrustabl } from './install'; import { runScan } from './runner'; import { readiness, risk, maxSeverity, severityCounts, projectedReadiness } from './score'; import { evaluateGate } from './gate'; -import { repoLabel, resolveBranch } from './git'; +import { repoLabel, resolveBranch, isRemoteTarget } from './git'; import { ReportData } from './report/model'; import { renderConsole } from './report/console'; import { buildSummaryMarkdown, writeStepSummary } from './report/summary'; @@ -90,13 +90,23 @@ async function run(): Promise { emitAnnotations(result.findings, inputs.maxAnnotations); } + // A remote URL target scans a different repo than this checkout, so the + // caller-repo-scoped surfaces (Code Scanning upload, PR comment) would + // misattribute results to this repo — skip them. + const remoteTarget = isRemoteTarget(inputs.target); + // Surface 1b: SARIF → Security tab (needs security-events: write). - if (inputs.uploadSarif) { + if (inputs.uploadSarif && !remoteTarget) { const res = await uploadSarif(inputs.githubToken, ctx, inputs.sarifFile); core.setOutput('sarif-uploaded', String(res.uploaded)); if (res.uploaded) core.info('Uploaded SARIF to Code Scanning.'); else if (res.reason) core.warning(res.reason); } else { + if (inputs.uploadSarif && remoteTarget) { + core.info( + 'Skipping Code Scanning upload: remote target — SARIF paths/commit do not match this repository.', + ); + } core.setOutput('sarif-uploaded', 'false'); } @@ -106,8 +116,9 @@ async function run(): Promise { await uploadResults(inputs.artifactName, [inputs.jsonFile, inputs.sarifFile], days); } - // Surface 2: sticky PR comment (needs pull-requests: write; pull_request only). - if (inputs.commentOnPr && ctx.isPullRequest) { + // Surface 2: sticky PR comment (needs pull-requests: write; pull_request only; + // skipped for a remote target — the comment would describe a different repo). + if (inputs.commentOnPr && ctx.isPullRequest && !remoteTarget) { await upsertComment(inputs.githubToken, ctx, md); } diff --git a/src/report/console.ts b/src/report/console.ts index 0eeff27..6c57c2c 100644 --- a/src/report/console.ts +++ b/src/report/console.ts @@ -55,7 +55,7 @@ export function buildConsoleLines(d: ReportData): string[] { L.push(row(`Max severity: ${d.maxSeverity} Native exit: ${d.nativeExit}`)); L.push(rule()); if (d.projected) { - L.push("Projected = estimate from trustabl's own scoring; listed fixes resolved, nothing new. Not a re-scan."); + L.push('Projected = estimate (listed fixes resolved), not a re-scan.'); } return L; }