diff --git a/migrations/checks/README.md b/migrations/checks/README.md index e0eb5de7..f718f518 100644 --- a/migrations/checks/README.md +++ b/migrations/checks/README.md @@ -263,6 +263,20 @@ steps: drift-resolution-scripts-name: flyway-drift-resolution-scripts-${{ matrix.target }} ``` +### Code Scanning + +When using Flyway 12.2+, code review results are automatically uploaded to [GitHub Code Scanning](https://docs.github.com/en/code-security/code-scanning) in SARIF format. Results appear in the Security tab and as PR annotations. + +This requires: +- GitHub Advanced Security to be enabled on the repository (available by default on public repos, requires GitHub Enterprise for private repos) +- The workflow to have `security-events: write` permission + +If either requirement is not met, the upload is silently skipped. + +| Input | Description | Required | Default | +|------------------------------|--------------------------------------------------------|----------|---------| +| `skip-code-scanning-upload` | Skip uploading SARIF results to GitHub Code Scanning | No | `false` | + ### Other | Input | Description | Required | Default | diff --git a/migrations/checks/action.yml b/migrations/checks/action.yml index 2d884d0c..ef43ade7 100644 --- a/migrations/checks/action.yml +++ b/migrations/checks/action.yml @@ -102,6 +102,10 @@ inputs: description: 'Number of days to retain the drift resolution scripts artifact' required: false default: '7' + skip-code-scanning-upload: + description: 'Skip uploading SARIF results to GitHub Code Scanning' + required: false + default: 'false' outputs: exit-code: @@ -159,3 +163,10 @@ runs: path: ${{ steps.checks.outputs.drift-resolution-folder }} retention-days: ${{ inputs.drift-resolution-scripts-retention-days }} if-no-files-found: ignore + - name: Upload SARIF to GitHub Code Scanning + if: always() && inputs.skip-code-scanning-upload == 'false' && steps.checks.outputs.sarif-path != '' + uses: github/codeql-action/upload-sarif@v3 + continue-on-error: true + with: + sarif_file: ${{ steps.checks.outputs.sarif-path }} + category: flyway-code-review diff --git a/migrations/checks/dist/index.js b/migrations/checks/dist/index.js index f984ff8e..1043444b 100644 --- a/migrations/checks/dist/index.js +++ b/migrations/checks/dist/index.js @@ -10584,6 +10584,7 @@ var un = () => { exitCode: n.exitCode, result: { reportPath: e?.error?.htmlReport, + sarifReportPath: e?.error?.sarifReport, ...t } }; @@ -10593,6 +10594,7 @@ var un = () => { exitCode: n.exitCode, result: { reportPath: r?.htmlReport, + sarifReportPath: r?.sarifReport, ...i } }; @@ -10623,6 +10625,7 @@ var un = () => { return rn("code-violation-count", r.violationCount.toString()), rn("code-violation-codes", r.violationCodes.join(",")), { exitCode: n, reportPath: r.reportPath, + sarifReportPath: r.sarifReportPath, violationCount: r.violationCount }; }, zn = async (e, t) => { @@ -10731,21 +10734,23 @@ var un = () => { a ], s = o.find((e) => e?.reportPath)?.reportPath; rn("report-path", vn(s ?? "report.html", e.workingDirectory)); - let c = o.find((e) => e !== void 0 && e.exitCode !== 0); - rn("exit-code", (c?.exitCode ?? 0).toString()); - let l; - if (i && (l = i.driftDetected ? "Drift detected" : i.comparisonSupported ? i.driftCheckSkipped ? "Drift check not run - skipped because no snapshot in database (expected for initial deployment)" : "No drift" : "Drift check not run - drift analysis is not supported for this database type"), await wn({ + let c = r?.sarifReportPath; + rn("sarif-path", c ? vn(c, e.workingDirectory) : ""); + let l = o.find((e) => e !== void 0 && e.exitCode !== 0); + rn("exit-code", (l?.exitCode ?? 0).toString()); + let u; + if (i && (u = i.driftDetected ? "Drift detected" : i.comparisonSupported ? i.driftCheckSkipped ? "Drift check not run - skipped because no snapshot in database (expected for initial deployment)" : "No drift" : "Drift check not run - drift analysis is not supported for this database type"), await wn({ dryrun: n ? { exitCode: n.exitCode } : void 0, code: r ? { exitCode: r.exitCode, violationCount: r.violationCount } : void 0, - driftStatus: l, + driftStatus: u, changes: a ? { exitCode: a.exitCode, changedObjectCount: a.changedObjectCount ?? 0 } : void 0 - }), c) throw Error("Flyway checks failed"); + }), l) throw Error("Flyway checks failed"); }, Gn = () => { let e = tn("target-environment") || void 0, t = tn("target-url") || void 0, n = tn("target-user") || void 0, r = tn("target-password") || void 0, i = tn("target-schemas") || void 0, a = tn("target-migration-version") || void 0, o = tn("cherry-pick") || void 0, s = tn("build-environment") || void 0, l = tn("build-url") || void 0, u = tn("build-user") || void 0, d = tn("build-password") || void 0, f = tn("build-schemas") || void 0, p = nn("build-ok-to-erase"), m = nn("skip-code-review"), h = nn("skip-drift-check"), g = nn("skip-deployment-changes-report"), _ = nn("skip-deployment-script-review"), v = nn("fail-on-code-review"), y = nn("fail-on-drift"), b = tn("working-directory"); return { diff --git a/migrations/checks/src/flyway/check-code.ts b/migrations/checks/src/flyway/check-code.ts index 27deb514..8126fec1 100644 --- a/migrations/checks/src/flyway/check-code.ts +++ b/migrations/checks/src/flyway/check-code.ts @@ -24,7 +24,12 @@ const runCheckCode = async (inputs: FlywayMigrationsChecksInputs) => { const { exitCode, result } = await checkForCodeReviewViolations(args, inputs.workingDirectory); core.setOutput("code-violation-count", result.violationCount.toString()); core.setOutput("code-violation-codes", result.violationCodes.join(",")); - return { exitCode, reportPath: result.reportPath, violationCount: result.violationCount }; + return { + exitCode, + reportPath: result.reportPath, + sarifReportPath: result.sarifReportPath, + violationCount: result.violationCount, + }; }; export { runCheckCode }; diff --git a/migrations/checks/src/flyway/run-checks.ts b/migrations/checks/src/flyway/run-checks.ts index 2c622c2c..ea88bb62 100644 --- a/migrations/checks/src/flyway/run-checks.ts +++ b/migrations/checks/src/flyway/run-checks.ts @@ -18,6 +18,9 @@ const runChecks = async (inputs: FlywayMigrationsChecksInputs, edition: FlywayEd const reportFile = results.find((r) => r?.reportPath)?.reportPath; core.setOutput("report-path", resolvePath(reportFile ?? "report.html", inputs.workingDirectory)); + const sarifFile = codeResult?.sarifReportPath; + core.setOutput("sarif-path", sarifFile ? resolvePath(sarifFile, inputs.workingDirectory) : ""); + const failed = results.find((r) => r !== undefined && r.exitCode !== 0); core.setOutput("exit-code", (failed?.exitCode ?? 0).toString()); diff --git a/migrations/checks/tests/flyway/check-code.test.ts b/migrations/checks/tests/flyway/check-code.test.ts index 2dcde809..34a59f51 100644 --- a/migrations/checks/tests/flyway/check-code.test.ts +++ b/migrations/checks/tests/flyway/check-code.test.ts @@ -98,15 +98,36 @@ describe("runCheckCode", () => { expect(args).not.toContain(expect.stringContaining("default_build")); }); - it("should return exitCode and reportPath from result", async () => { + it("should return exitCode, reportPath, and sarifReportPath from result", async () => { checkForCodeReviewViolations.mockResolvedValue({ exitCode: 1, - result: { reportPath: "/tmp/report.html", violationCount: 1, violationCodes: ["RG06"] }, + result: { + reportPath: "/tmp/report.html", + sarifReportPath: "/tmp/report.sarif", + violationCount: 1, + violationCodes: ["RG06"], + }, }); const result = await runCheckCode({}); - expect(result).toEqual({ exitCode: 1, reportPath: "/tmp/report.html", violationCount: 1 }); + expect(result).toEqual({ + exitCode: 1, + reportPath: "/tmp/report.html", + sarifReportPath: "/tmp/report.sarif", + violationCount: 1, + }); + }); + + it("should return undefined sarifReportPath when not present in result", async () => { + checkForCodeReviewViolations.mockResolvedValue({ + exitCode: 0, + result: { reportPath: "/tmp/report.html", violationCount: 0, violationCodes: [] }, + }); + + const result = await runCheckCode({}); + + expect(result).toEqual(expect.objectContaining({ sarifReportPath: undefined })); }); it("should set code-violation-count and code-violation-codes outputs", async () => { diff --git a/migrations/checks/tests/flyway/run-checks.test.ts b/migrations/checks/tests/flyway/run-checks.test.ts index 6b6ddaa6..749bf6b3 100644 --- a/migrations/checks/tests/flyway/run-checks.test.ts +++ b/migrations/checks/tests/flyway/run-checks.test.ts @@ -195,4 +195,28 @@ describe("runChecks", () => { expect(setOutput).toHaveBeenCalledWith("report-path", "/tmp/reports/custom-report.html"); }); + + it("should set sarif-path from sarifReport in code review output", async () => { + exec.mockImplementation(mockExec({ stdout: { sarifReport: "report.sarif" } })); + + await runChecks(baseInputs, "enterprise"); + + expect(setOutput).toHaveBeenCalledWith("sarif-path", "report.sarif"); + }); + + it("should set sarif-path to empty string when no sarifReport in output", async () => { + exec.mockImplementation(mockExec({ stdout: {} })); + + await runChecks(baseInputs, "enterprise"); + + expect(setOutput).toHaveBeenCalledWith("sarif-path", ""); + }); + + it("should prepend working directory to sarif-path", async () => { + exec.mockImplementation(mockExec({ stdout: { sarifReport: "report.sarif" } })); + + await runChecks({ workingDirectory: "my-project" }, "enterprise"); + + expect(setOutput).toHaveBeenCalledWith("sarif-path", path.join("my-project", "report.sarif")); + }); }); diff --git a/shared/src/check/check-for-code-review-violations.ts b/shared/src/check/check-for-code-review-violations.ts index 741a2c28..4642be9f 100644 --- a/shared/src/check/check-for-code-review-violations.ts +++ b/shared/src/check/check-for-code-review-violations.ts @@ -5,17 +5,25 @@ type CodeResultItem = { violations?: { code?: string }[] }; type CodeReviewSuccessOutput = { htmlReport?: string; + sarifReport?: string; individualResults?: { operation?: string; results?: CodeResultItem[] }[]; }; type CodeReviewErrorOutput = { - error?: { errorCode?: string; message?: string; results?: CodeResultItem[]; htmlReport?: string }; + error?: { + errorCode?: string; + message?: string; + results?: CodeResultItem[]; + htmlReport?: string; + sarifReport?: string; + }; }; type CheckForCodeReviewResult = { exitCode: number; result: { reportPath?: string; + sarifReportPath?: string; violationCount: number; violationCodes: string[]; }; @@ -33,14 +41,24 @@ const checkForCodeReviewViolations = async ( const errorOutput = parseOutput(result.stdout); errorOutput?.error?.message && core.error(errorOutput.error.message); const violations = extractViolations(errorOutput?.error?.results ?? []); - return { exitCode: result.exitCode, result: { reportPath: errorOutput?.error?.htmlReport, ...violations } }; + return { + exitCode: result.exitCode, + result: { + reportPath: errorOutput?.error?.htmlReport, + sarifReportPath: errorOutput?.error?.sarifReport, + ...violations, + }, + }; } const output = parseOutput(result.stdout); const codeResults = output?.individualResults?.filter((r) => r.operation === "code"); const resultItems = codeResults?.flatMap((r) => r.results ?? []) ?? []; const violations = extractViolations(resultItems); - return { exitCode: result.exitCode, result: { reportPath: output?.htmlReport, ...violations } }; + return { + exitCode: result.exitCode, + result: { reportPath: output?.htmlReport, sarifReportPath: output?.sarifReport, ...violations }, + }; } finally { core.endGroup(); } diff --git a/shared/tests/check/check-for-code-review-violations.test.ts b/shared/tests/check/check-for-code-review-violations.test.ts index 9304535c..64c0b57f 100644 --- a/shared/tests/check/check-for-code-review-violations.test.ts +++ b/shared/tests/check/check-for-code-review-violations.test.ts @@ -36,6 +36,7 @@ describe("checkForCodeReviewViolations", () => { mockExec({ stdout: { htmlReport: "report.html", + sarifReport: "report.sarif", individualResults: [ { operation: "code", @@ -52,6 +53,7 @@ describe("checkForCodeReviewViolations", () => { exitCode: 0, result: { reportPath: "report.html", + sarifReportPath: "report.sarif", violationCount: 3, violationCodes: ["AM04", "RG06"], }, @@ -67,6 +69,7 @@ describe("checkForCodeReviewViolations", () => { message: "Code Analysis Violation(s) detected", results: [{ violations: [{ code: "AM04" }] }], htmlReport: "/tmp/report.html", + sarifReport: "/tmp/report.sarif", }, }, exitCode: 1, @@ -79,6 +82,7 @@ describe("checkForCodeReviewViolations", () => { exitCode: 1, result: { reportPath: "/tmp/report.html", + sarifReportPath: "/tmp/report.sarif", violationCount: 1, violationCodes: ["AM04"], }, diff --git a/state/prepare/README.md b/state/prepare/README.md index 660e0bbc..46c457cd 100644 --- a/state/prepare/README.md +++ b/state/prepare/README.md @@ -170,6 +170,20 @@ steps: undo-script-name: flyway-undo-script-${{ matrix.target }} ``` +### Code Scanning + +When using Flyway 12.2+, code review results are automatically uploaded to [GitHub Code Scanning](https://docs.github.com/en/code-security/code-scanning) in SARIF format. Results appear in the Security tab and as PR annotations. + +This requires: +- GitHub Advanced Security to be enabled on the repository (available by default on public repos, requires GitHub Enterprise for private repos) +- The workflow to have `security-events: write` permission + +If either requirement is not met, the upload is silently skipped. + +| Input | Description | Required | Default | +|------------------------------|--------------------------------------------------------|----------|---------| +| `skip-code-scanning-upload` | Skip uploading SARIF results to GitHub Code Scanning | No | `false` | + ## Outputs | Output | Description | diff --git a/state/prepare/action.yml b/state/prepare/action.yml index e7d86997..de0b4703 100644 --- a/state/prepare/action.yml +++ b/state/prepare/action.yml @@ -91,6 +91,10 @@ inputs: description: 'Number of days to retain the deployment and undo script artifacts' required: false default: '7' + skip-code-scanning-upload: + description: 'Skip uploading SARIF results to GitHub Code Scanning' + required: false + default: 'false' outputs: exit-code: @@ -171,3 +175,10 @@ runs: archive: false retention-days: ${{ inputs.deployment-script-retention-days }} if-no-files-found: ignore + - name: Upload SARIF to GitHub Code Scanning + if: always() && inputs.skip-code-scanning-upload == 'false' && steps.prepare.outputs.sarif-path != '' + uses: github/codeql-action/upload-sarif@v3 + continue-on-error: true + with: + sarif_file: ${{ steps.prepare.outputs.sarif-path }} + category: flyway-code-review diff --git a/state/prepare/dist/index.js b/state/prepare/dist/index.js index 06a207d9..46c046f3 100644 --- a/state/prepare/dist/index.js +++ b/state/prepare/dist/index.js @@ -10527,6 +10527,7 @@ var dn = () => { exitCode: n.exitCode, result: { reportPath: e?.error?.htmlReport, + sarifReportPath: e?.error?.sarifReport, ...t } }; @@ -10536,6 +10537,7 @@ var dn = () => { exitCode: n.exitCode, result: { reportPath: r?.htmlReport, + sarifReportPath: r?.sarifReport, ...i } }; @@ -10549,6 +10551,8 @@ var dn = () => { violationCodes: [...new Set(t)] }; }, kn = (e, t) => { + if (e) return f.isAbsolute(e) ? e : t ? f.join(t, e) : e; +}, An = (e, t) => { if (e.skipCodeReview) { cn("Skipping code review: \"skip-code-review\" set to true"); return; @@ -10564,12 +10568,14 @@ var dn = () => { "-check.scope=script", `-check.scriptFilename=${t}` ]; -}, An = async (e, t) => { - let n = kn(e, t); +}, jn = async (e, t) => { + let n = An(e, t); if (!n) return; let r = await Dn(n, e.workingDirectory); - return rn("code-violation-count", r.result.violationCount.toString()), rn("code-violation-codes", r.result.violationCodes.join(",")), r; -}, jn = async (e, t) => { + rn("code-violation-count", r.result.violationCount.toString()), rn("code-violation-codes", r.result.violationCodes.join(",")); + let i = r.result.sarifReportPath; + return rn("sarif-path", i ? kn(i, e.workingDirectory) : ""), r; +}, Mn = async (e, t) => { ln("Checking for drift"); try { let n = await gn(e, t); @@ -10608,8 +10614,6 @@ var dn = () => { } finally { un(); } -}, Mn = (e, t) => { - if (e) return f.isAbsolute(e) ? e : t ? f.join(t, e) : e; }, Nn = (e) => [ "check", "-drift", @@ -10617,7 +10621,7 @@ var dn = () => { ...Cn(e), ...e.preDeploymentReportName ? [`-reportFilename=${e.preDeploymentReportName}`] : [] ], Pn = async (e) => { - let { exitCode: t, result: n } = await jn(Nn(e), e.workingDirectory), r = Mn(n.reportPath, e.workingDirectory), i = Mn(n.driftResolutionFolder, e.workingDirectory); + let { exitCode: t, result: n } = await Mn(Nn(e), e.workingDirectory), r = kn(n.reportPath, e.workingDirectory), i = kn(n.driftResolutionFolder, e.workingDirectory); return rn("exit-code", t.toString()), n.driftDetected !== void 0 && rn("drift-detected", n.driftDetected.toString()), r !== void 0 && rn("report-path", r), i !== void 0 && rn("drift-resolution-folder", i), { exitCode: t, result: n @@ -10752,7 +10756,7 @@ await (async () => { }); return; } - let a = await An(t, i); + let a = await jn(t, i); if (await Un({ driftStatus: n, code: a ? { diff --git a/state/prepare/src/flyway/check-code.ts b/state/prepare/src/flyway/check-code.ts index 942f488b..9c4c768d 100644 --- a/state/prepare/src/flyway/check-code.ts +++ b/state/prepare/src/flyway/check-code.ts @@ -2,6 +2,7 @@ import type { FlywayStatePrepareInputs } from "../types.js"; import * as core from "@actions/core"; import { checkForCodeReviewViolations } from "@flyway-actions/shared/check-for-code-review-violations"; import { parseExtraArgs } from "@flyway-actions/shared/flyway-runner"; +import { resolvePath } from "@flyway-actions/shared/resolve-path"; import { getTargetEnvironmentArgs } from "./arg-builders.js"; const getCodeArgs = (inputs: FlywayStatePrepareInputs, scriptFilename: string): string[] | undefined => { @@ -30,6 +31,8 @@ const runCheckCode = async (inputs: FlywayStatePrepareInputs, scriptFilename: st const codeReviewResult = await checkForCodeReviewViolations(args, inputs.workingDirectory); core.setOutput("code-violation-count", codeReviewResult.result.violationCount.toString()); core.setOutput("code-violation-codes", codeReviewResult.result.violationCodes.join(",")); + const sarifPath = codeReviewResult.result.sarifReportPath; + core.setOutput("sarif-path", sarifPath ? resolvePath(sarifPath, inputs.workingDirectory) : ""); return codeReviewResult; }; diff --git a/state/prepare/tests/flyway/check-code.test.ts b/state/prepare/tests/flyway/check-code.test.ts index 8b29b4bc..39a8debb 100644 --- a/state/prepare/tests/flyway/check-code.test.ts +++ b/state/prepare/tests/flyway/check-code.test.ts @@ -132,4 +132,37 @@ describe("runCheckCode", () => { expect(setOutput).toHaveBeenCalledWith("code-violation-count", "3"); expect(setOutput).toHaveBeenCalledWith("code-violation-codes", "AM04,RG06"); }); + + it("should set sarif-path output when sarifReportPath is present", async () => { + checkForCodeReviewViolations.mockResolvedValue({ + exitCode: 0, + result: { sarifReportPath: "report.sarif", violationCount: 0, violationCodes: [] }, + }); + + await runCheckCode({}, "V001__create.sql"); + + expect(setOutput).toHaveBeenCalledWith("sarif-path", "report.sarif"); + }); + + it("should set sarif-path to empty string when sarifReportPath is absent", async () => { + checkForCodeReviewViolations.mockResolvedValue({ + exitCode: 0, + result: { violationCount: 0, violationCodes: [] }, + }); + + await runCheckCode({}, "V001__create.sql"); + + expect(setOutput).toHaveBeenCalledWith("sarif-path", ""); + }); + + it("should prepend working directory to sarif-path", async () => { + checkForCodeReviewViolations.mockResolvedValue({ + exitCode: 0, + result: { sarifReportPath: "report.sarif", violationCount: 0, violationCodes: [] }, + }); + + await runCheckCode({ workingDirectory: "my-project" }, "V001__create.sql"); + + expect(setOutput).toHaveBeenCalledWith("sarif-path", expect.stringContaining("my-project")); + }); });