fix(desktop): run git describe without a shell so version derivation works on Windows (#5097)

#5057 restricted version derivation to `git describe --tags --match 'v[0-9]*'`,
but the command was passed to `execSync` as a shell string. On Windows the
shell is cmd.exe, which does not strip the POSIX single quotes around
'v[0-9]*', so git received the quotes literally, matched no tag, fell through
to `--always`, and the version degraded to the `0.0.0-g<hash>` fallback.

That is what shipped a `0.0.0-gc05b67ae4` Windows Desktop build (electron-builder
`--publish always` then auto-created a bogus release) during the v0.3.41 release,
even though the tag was sitting exactly on HEAD. Linux/macOS were unaffected
because /bin/sh strips the quotes.

Fix: invoke git with an argv array via execFileSync in every version-derivation
path, so the match pattern reaches git as one literal argument regardless of
platform:

- apps/desktop/scripts/package.mjs      (Desktop version → electron-builder)
- apps/desktop/scripts/bundle-cli.mjs   (bundled CLI ldflags version)
- apps/desktop/src/main/app-version.ts  (dev-mode version fallback)

The Makefile is intentionally left as-is: make's `$(shell ...)` always runs via
/bin/sh (even on Windows) and the CLI release runs on Linux, so its single
quotes are stripped correctly.

Tests: export `deriveVersion` and `DESCRIBE_ARGS` and add coverage that runs the
real `git describe` against throwaway repos (clean semver tag, semver tag chosen
over a nearer non-semver tag, and the no-tag fallback), plus a structural check
that the match pattern is a bare argv token with no embedded quotes. The prior
suite only unit-tested the `normalizeGitVersion` string transform, which is why
this slipped through.

MUL-4256

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
Bohan Jiang
2026-07-08 18:49:46 +08:00
committed by GitHub
parent c05b67ae4a
commit 3790ca78e7
4 changed files with 132 additions and 21 deletions

View File

@@ -74,9 +74,14 @@ const srcBinary = join(serverDir, "bin", `${goos}-${goarch}`, binName);
const destDir = join(repoRoot, "apps", "desktop", "resources", "bin");
const destBinary = join(destDir, binName);
function sh(cmd) {
// Hand git arguments straight to the binary (no shell). A match pattern like
// `v[0-9]*` must reach git as one literal argument; routing it through a shell
// string breaks on Windows, where cmd.exe keeps the POSIX single quotes and
// git matches no tag — degrading the bundled CLI's version to the
// 0.0.0-g<hash> fallback.
function git(...args) {
try {
return execSync(cmd, { encoding: "utf-8" }).trim();
return execFileSync("git", args, { encoding: "utf-8" }).trim();
} catch {
return "";
}
@@ -101,8 +106,10 @@ async function exists(p) {
}
if (hasGo()) {
const version = sh("git describe --tags --match 'v[0-9]*' --always --dirty") || "dev";
const commit = sh("git rev-parse --short HEAD") || "unknown";
const version =
git("describe", "--tags", "--match", "v[0-9]*", "--always", "--dirty") ||
"dev";
const commit = git("rev-parse", "--short", "HEAD") || "unknown";
const date = new Date().toISOString().replace(/\.\d+Z$/, "Z");
const ldflags = `-X main.version=${version} -X main.commit=${commit} -X main.date=${date}`;

View File

@@ -22,10 +22,11 @@
// build, set `CSC_IDENTITY_AUTO_DISCOVERY=false` so electron-builder falls
// back to an ad-hoc signature instead of requiring a Developer ID cert.
//
// The `normalizeGitVersion` helper is exported so tests can cover the
// version-derivation logic without shelling out.
// The `normalizeGitVersion`, `deriveVersion`, and `DESCRIBE_ARGS` exports let
// tests cover version derivation both as a pure string transform and as the
// real `git describe` invocation against a throwaway repo.
import { execFileSync, spawnSync, execSync } from "node:child_process";
import { execFileSync, spawnSync } from "node:child_process";
import { delimiter, dirname, resolve } from "node:path";
import { fileURLToPath, pathToFileURL } from "node:url";
@@ -71,9 +72,17 @@ const MAC_ALL_PLATFORM_TARGETS = [
{ platform: "linux", arch: "arm64" },
];
function sh(cmd) {
// Run a git subcommand with its arguments handed straight to the binary,
// never through a shell. A match pattern like `v[0-9]*` must reach git as a
// single literal argument on every platform. Passing the whole command as a
// shell string (execSync) is unsafe on Windows: cmd.exe does not strip the
// POSIX single quotes around 'v[0-9]*', so git receives the quotes verbatim,
// matches no tag, and the version silently degrades to the 0.0.0-g<hash>
// fallback — which is exactly how a `0.0.0-…` Windows Desktop build once
// escaped to a GitHub Release.
function git(args, cwd) {
try {
return execSync(cmd, { encoding: "utf-8" }).trim();
return execFileSync("git", args, { encoding: "utf-8", cwd }).trim();
} catch {
return "";
}
@@ -126,10 +135,24 @@ export function normalizeGitVersion(raw) {
return stripped;
}
function deriveVersion() {
return normalizeGitVersion(
sh("git describe --tags --match 'v[0-9]*' --always --dirty"),
);
// The exact argv handed to `git describe` for version derivation. Kept as a
// standalone array — never a shell command string — so the `v[0-9]*` match
// pattern reaches git as one literal argument regardless of platform (see the
// git() note above for why a shell string breaks on Windows).
export const DESCRIBE_ARGS = [
"describe",
"--tags",
"--match",
"v[0-9]*",
"--always",
"--dirty",
];
// Exported (with an optional cwd) so tests can exercise the real describe
// invocation against a throwaway repo, not just normalizeGitVersion in
// isolation — the gap that let the Windows quoting regression through CI.
export function deriveVersion(cwd) {
return normalizeGitVersion(git(DESCRIBE_ARGS, cwd));
}
function uniqueOrdered(values) {

View File

@@ -1,7 +1,12 @@
import { delimiter, resolve } from "node:path";
import { describe, it, expect } from "vitest";
import { execFileSync } from "node:child_process";
import { mkdtempSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { delimiter, join, resolve } from "node:path";
import { afterEach, describe, it, expect } from "vitest";
import {
builderArgsForTarget,
deriveVersion,
DESCRIBE_ARGS,
envWithLocalBins,
normalizeGitVersion,
parsePackageArgs,
@@ -79,6 +84,75 @@ describe("normalizeGitVersion", () => {
});
});
describe("DESCRIBE_ARGS", () => {
it("passes the match pattern as one bare argv token, never a shell-quoted string", () => {
// The Windows regression this locks down: the pattern used to be embedded
// in a shell command string as `--match 'v[0-9]*'`. cmd.exe does not strip
// POSIX single quotes, so git received them literally and matched no tag,
// collapsing the Desktop version to the 0.0.0-g<hash> fallback. As a
// standalone argv element with no surrounding quotes the pattern is
// shell-independent.
expect(DESCRIBE_ARGS).toContain("v[0-9]*");
for (const arg of DESCRIBE_ARGS) {
expect(arg).not.toContain("'");
expect(arg).not.toContain('"');
}
});
});
describe("deriveVersion (real git describe)", () => {
// These exercise the actual `git describe` invocation — not just the
// normalizeGitVersion string transform — because the bug that shipped a
// `0.0.0-…` Windows Desktop build lived in HOW git was called, not in the
// string handling. package.mjs now runs git with an argv array (no shell),
// so the `v[0-9]*` match pattern reaches git as a literal argument
// identically on every platform.
const repos = [];
function initRepo() {
const dir = mkdtempSync(join(tmpdir(), "multica-desktop-ver-"));
repos.push(dir);
const run = (...args) =>
execFileSync("git", args, { cwd: dir, encoding: "utf-8" });
run("init", "-q");
run("config", "user.email", "test@multica.ai");
run("config", "user.name", "test");
run("config", "commit.gpgsign", "false");
run("commit", "-q", "--allow-empty", "-m", "root");
return { dir, run };
}
afterEach(() => {
while (repos.length) rmSync(repos.pop(), { recursive: true, force: true });
});
it("resolves a clean semver tag to its bare version", () => {
const { dir, run } = initRepo();
run("tag", "v1.4.2");
expect(deriveVersion(dir)).toBe("1.4.2");
});
it("selects the semver tag even when a nearer non-semver tag exists", () => {
// A release-train tag like `release_iteration/…` sitting closer to HEAD
// must not become the version. With the match pattern correctly reaching
// git, describe skips it and reports the real vX.Y.Z tag. If the pattern
// were mangled (e.g. quotes leaking through a shell) git would match
// nothing and the version would collapse to `0.0.0-…`.
const { dir, run } = initRepo();
run("tag", "v1.4.2");
run("commit", "-q", "--allow-empty", "-m", "sprint");
run("tag", "release_iteration/Sprint_0705");
const version = deriveVersion(dir);
expect(version).toMatch(/^1\.4\.2-1-g[0-9a-f]+$/);
expect(version).not.toMatch(/^0\.0\.0/);
});
it("falls back to 0.0.0-g<hash> when no semver tag is reachable", () => {
const { dir } = initRepo();
expect(deriveVersion(dir)).toMatch(/^0\.0\.0-g[0-9a-f]+$/);
});
});
describe("stripLeadingSeparator", () => {
it("removes the leading -- inserted by npm/pnpm", () => {
expect(stripLeadingSeparator(["--", "--mac", "--arm64", "--publish", "always"])).toEqual([

View File

@@ -1,5 +1,5 @@
import { app } from "electron";
import { execSync } from "node:child_process";
import { execFileSync } from "node:child_process";
/**
* Resolve the running app version. In packaged builds this is the value
@@ -20,11 +20,18 @@ export function getAppVersion(): string {
return app.getVersion();
}
try {
const raw = execSync("git describe --tags --match 'v[0-9]*' --always --dirty", {
cwd: app.getAppPath(),
encoding: "utf-8",
stdio: ["ignore", "pipe", "ignore"],
}).trim();
// argv array, not a shell string: the `v[0-9]*` match pattern must reach
// git as one literal argument. A shell string breaks on Windows, where
// cmd.exe keeps the single quotes and git matches no tag.
const raw = execFileSync(
"git",
["describe", "--tags", "--match", "v[0-9]*", "--always", "--dirty"],
{
cwd: app.getAppPath(),
encoding: "utf-8",
stdio: ["ignore", "pipe", "ignore"],
},
).trim();
if (!raw) return app.getVersion();
return raw.replace(/^v/, "");
} catch {