Merge pull request #76 from koji-1009/security-hardening

Security hardening

Author: koji-1009

.github/workflows/ci.yml

@@ -13,6 +13,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
+        with:
+          persist-credentials: false
       - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
         with:
           node-version: 24.x

.github/workflows/e2e.yml

@@ -21,6 +21,8 @@ jobs:
     runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
+        with:
+          persist-credentials: false
 
       - name: Setup Flutter (stable latest)
         uses: ./
@@ -40,6 +42,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
+        with:
+          persist-credentials: false
 
       - name: Setup Flutter (exact version)
         uses: ./
@@ -55,6 +59,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
+        with:
+          persist-credentials: false
 
       - name: Setup Flutter (dry-run)
         id: flutter
@@ -63,7 +69,10 @@ jobs:
           dry-run: true
 
       - name: Verify outputs
+        env:
+          FLUTTER_VERSION: ${{ steps.flutter.outputs.flutter-version }}
+          CHANNEL: ${{ steps.flutter.outputs.channel }}
         run: |
-          echo "Flutter version: ${{ steps.flutter.outputs.flutter-version }}"
-          echo "Channel: ${{ steps.flutter.outputs.channel }}"
-          test -n "${{ steps.flutter.outputs.flutter-version }}"
+          echo "Flutter version: $FLUTTER_VERSION"
+          echo "Channel: $CHANNEL"
+          test -n "$FLUTTER_VERSION"

README.md

@@ -168,6 +168,16 @@ The action sets the following outputs:
 | `cache-pub-hit`   | Whether pub cache was restored                    |
 | `architecture`    | Resolved CPU architecture                         |
 
+## Security
+
+The intro bullets above are backed by a consistent design: every artifact this action installs is verified, and no input ever reaches a shell.
+
+* **Archive verification** — Release archives are downloaded over HTTPS and verified against the SHA-256 checksum in the official release manifest before extraction. A mismatch fails the run.
+* **Git mode verification** — Versions and refs are resolved to a commit hash first (via the release manifest or `git ls-remote`), and after cloning the checkout is verified to match that commit. SDK caches are keyed by the commit hash and the repository URL.
+* **No shell interpolation** — Inputs and version-file contents are passed to subprocesses as argument arrays. Nothing is ever interpolated into a shell command.
+* **Supply chain** — All runtime dependencies are bundled into `dist/`, and CI verifies the bundle is reproducible from source on every change. The repository's own workflows are SHA-pinned, run with minimal permissions, and use `npm ci --ignore-scripts` wherever a write token is present.
+* **Cache integrity** — Caches are restored by exact key match only. There is no partial-match fallback that could restore unexpected content.
+
 ## Blog Post
 
 [Why I Built a New GitHub Action for Flutter](https://koji-1009.medium.com/why-i-built-a-new-github-action-for-flutter-592c24e96a55) — design decisions and motivation behind this action.

__tests__/git-source.test.ts

@@ -538,9 +538,22 @@ describe("resolveGit (dispatch)", () => {
 });
 
 describe("installFromGit", () => {
+	const FULL_HASH = "abc123def4567890abc123def4567890abc123de";
+
+	// Echo FULL_HASH for `git rev-parse HEAD` so the post-clone verification
+	// passes by default; tests override this to simulate a mismatch.
+	function mockExecWithHead(head: string) {
+		vi.mocked(exec).mockImplementation(async (_cmd, args, options) => {
+			if (args?.includes("rev-parse")) {
+				options?.listeners?.stdout?.(Buffer.from(`${head}\n`));
+			}
+			return 0;
+		});
+	}
+
 	beforeEach(() => {
 		vi.clearAllMocks();
-		vi.mocked(exec).mockResolvedValue(0);
+		mockExecWithHead(FULL_HASH);
 	});
 
 	const gitEnvMatcher = expect.objectContaining({
@@ -614,6 +627,46 @@ describe("installFromGit", () => {
 		);
 	});
 
+	it("verifies cloned HEAD against the resolved commit for branch ref", async () => {
+		await installFromGit(
+			"https://github.com/flutter/flutter.git",
+			"stable",
+			"/opt/flutter",
+			FULL_HASH,
+		);
+		expect(exec).toHaveBeenCalledWith(
+			"git",
+			["-C", "/opt/flutter", "rev-parse", "HEAD"],
+			expect.anything(),
+		);
+	});
+
+	it("throws when cloned HEAD does not match the resolved commit", async () => {
+		mockExecWithHead("1111111111111111111111111111111111111111");
+
+		await expect(
+			installFromGit(
+				"https://github.com/flutter/flutter.git",
+				"stable",
+				"/opt/flutter",
+				FULL_HASH,
+			),
+		).rejects.toThrow("does not match resolved commit");
+	});
+
+	it("skips HEAD verification for short hash commitHash", async () => {
+		await installFromGit(
+			"https://github.com/flutter/flutter.git",
+			"stable",
+			"/opt/flutter",
+			"abc1234",
+		);
+		const revParseCall = vi
+			.mocked(exec)
+			.mock.calls.find((c) => (c[1] as string[])?.includes("rev-parse"));
+		expect(revParseCall).toBeUndefined();
+	});
+
 	it("throws when command times out", async () => {
 		vi.useFakeTimers();
 		vi.mocked(exec).mockReturnValue(new Promise(() => {}));

__tests__/main.test.ts

@@ -241,6 +241,19 @@ describe("main run()", () => {
 		);
 	});
 
+	it("calls setFailed for an invalid git-source value", async () => {
+		const { inputs } = setupDefaultMocks();
+		inputs["git-source"] = "releases";
+
+		await run();
+
+		expect(setFailed).toHaveBeenCalledWith(
+			expect.stringContaining("Invalid git-source"),
+		);
+		expect(fetchManifest).not.toHaveBeenCalled();
+		expect(installFromArchive).not.toHaveBeenCalled();
+	});
+
 	it("uses git mode with ref spec", async () => {
 		const { inputs } = setupDefaultMocks();
 		inputs["git-source"] = "git";

__tests__/version.test.ts

@@ -19,6 +19,10 @@ import {
 } from "../src/version";
 
 vi.mock("@actions/http-client");
+vi.mock("@actions/core");
+vi.mock("node:timers/promises", () => ({
+	setTimeout: vi.fn().mockResolvedValue(undefined),
+}));
 
 const linuxFixture: FlutterManifest = JSON.parse(
 	readFileSync(join(__dirname, "fixtures", "releases_linux.json"), "utf8"),
@@ -133,6 +137,18 @@ describe("parseVersionSpec", () => {
 		});
 	});
 
+	it("throws for ambiguous major.minor form 3.4", () => {
+		expect(() => parseVersionSpec("3.4")).toThrow(
+			"Ambiguous version '3.4': use '3.4.x'",
+		);
+	});
+
+	it("throws for ambiguous bare major form 3", () => {
+		expect(() => parseVersionSpec("3")).toThrow(
+			"Ambiguous version '3': use '3.x'",
+		);
+	});
+
 	it("returns ref for branch name", () => {
 		expect(parseVersionSpec("my-feature-branch")).toEqual({
 			type: "ref",
@@ -601,14 +617,63 @@ describe("fetchManifest", () => {
 	});
 
 	it("throws when result is null", async () => {
+		const getJson = vi.fn().mockResolvedValue({ result: null });
 		HttpClient.mockImplementation(
 			class {
-				getJson = vi.fn().mockResolvedValue({ result: null });
+				getJson = getJson;
 			},
 		);
 
 		await expect(fetchManifest("linux")).rejects.toThrow(
 			"Failed to fetch manifest",
 		);
+		// A 404 (null result) must not be retried.
+		expect(getJson).toHaveBeenCalledTimes(1);
+	});
+
+	it("retries on HTTP 500 and succeeds on next attempt", async () => {
+		const getJson = vi
+			.fn()
+			.mockRejectedValueOnce(
+				Object.assign(new Error("Failed request: (500)"), { statusCode: 500 }),
+			)
+			.mockResolvedValue({ result: JSON.parse(JSON.stringify(linuxFixture)) });
+		HttpClient.mockImplementation(
+			class {
+				getJson = getJson;
+			},
+		);
+
+		const result = await fetchManifest("linux");
+		expect(result.releases.length).toBeGreaterThan(0);
+		expect(getJson).toHaveBeenCalledTimes(2);
+	});
+
+	it("retries on network error and throws after exhausting attempts", async () => {
+		const getJson = vi.fn().mockRejectedValue(new Error("ECONNRESET"));
+		HttpClient.mockImplementation(
+			class {
+				getJson = getJson;
+			},
+		);
+
+		await expect(fetchManifest("linux")).rejects.toThrow("ECONNRESET");
+		expect(getJson).toHaveBeenCalledTimes(3);
+	});
+
+	it("does not retry on HTTP 403", async () => {
+		const getJson = vi
+			.fn()
+			.mockRejectedValue(
+				Object.assign(new Error("Failed request: (403)"), { statusCode: 403 }),
+			);
+		HttpClient.mockImplementation(
+			class {
+				getJson = getJson;
+			},
+		);
+
+		await expect(fetchManifest("linux")).rejects.toThrow("(403)");
+		expect(getJson).toHaveBeenCalledTimes(1);
 	});
 });