From 8460c5485370fda972d98329bdf9f46d60b5c9dd Mon Sep 17 00:00:00 2001 From: Gustavo Will <48598429+gitgusilva@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:38:52 -0300 Subject: [PATCH 1/5] ci: verify builds on pull requests The packaging workflows only ran on a manual dispatch or a v* tag, so a change could reach main without ever having been built. On a tag push electron-builder publishes to the GitHub Release as it packages, which made the first real build of any change also its release. Run both on pull requests to main. Nothing publishes: the --publish flag already resolves to "never" for anything that is not a tag. The paths filter skips PRs that cannot affect the build (docs, site, images) so they do not spend ~20 minutes packaging installers nobody downloads. --- .github/workflows/build-linux.yml | 15 +++++++++++++++ .github/workflows/build-windows.yml | 11 +++++++++++ 2 files changed, 26 insertions(+) diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index 03da92e..1c62468 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -14,6 +14,21 @@ on: push: tags: - "v*" + # Pull requests build and verify but never publish (the --publish flag below + # only says "always" for a tag). The paths filter keeps doc-only PRs from + # spending ~20 min packaging four installers; note that a required status + # check on this workflow would block those PRs forever, so if you make it + # required, drop the filter. + pull_request: + branches: + - main + paths: + - "core/**" + - "app/**" + - "ui/**" + - "domain/**" + - "shared/**" + - ".github/workflows/build-linux.yml" # Needed for electron-builder to upload artifacts to the GitHub Release on a # tag push. The default GITHUB_TOKEN is read-only, so publishing fails with diff --git a/.github/workflows/build-windows.yml b/.github/workflows/build-windows.yml index 1812c57..41a0e05 100644 --- a/.github/workflows/build-windows.yml +++ b/.github/workflows/build-windows.yml @@ -15,6 +15,17 @@ on: push: tags: - "v*" + # Same rules as build-linux.yml: PRs build and verify, never publish. + pull_request: + branches: + - main + paths: + - "core/**" + - "app/**" + - "ui/**" + - "domain/**" + - "shared/**" + - ".github/workflows/build-windows.yml" # Needed for electron-builder to upload artifacts to the GitHub Release on a # tag push. The default GITHUB_TOKEN is read-only, so publishing fails with From 03aaf32be8b58dce1f0a549b0223c2ead332b73d Mon Sep 17 00:00:00 2001 From: Gustavo Will <48598429+gitgusilva@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:39:22 -0300 Subject: [PATCH 2/5] fix(addon): link the vendored static libcrypto for libssh2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitBox failed to start on Ubuntu 24.04 (v1.1.4 through v1.1.6): Error: .../gitbox_addon.node: undefined symbol: EVP_des_ede3_cbc Adding libssh2 for native SSH brought in its OpenSSL crypto backend, and the vendor script builds a static OpenSSL for it, but binding.gyp only listed libssh2.a. So ~97 OpenSSL symbols stayed undefined in the addon, and libssh2's crypt-method table kept them as data relocations (R_X86_64_64) that the loader must resolve at dlopen time. Nothing caught it. Linking a shared object with undefined symbols is legal, so the build passed; plain node exports the OpenSSL it links statically, so the addon quietly borrowed those symbols and worked in development. Electron ships BoringSSL and exports no EVP_des_ede3_cbc, so the packaged app only survived on machines that happened to have a compatible libcrypto already mapped into the process — which is why it ran on the maintainer's Fedora box and died on the reporter's Ubuntu. Link vendor/openssl/install/lib/libcrypto.a after libssh2.a, plus -ldl and -lpthread for OpenSSL's DSO code. The glibc floor is unchanged, and the dependency set stays libc/libstdc++/libm/libgcc. Add test/self-contained.test.js to make sure this cannot ship again. It audits the built addon's imported symbols instead of just loading it, because loading proves nothing: against a deliberately rebuilt broken addon the three audit tests fail while both load tests still pass on a host that has a system libcrypto. Both workflows now compile the addon and run the audit before packaging, since a tag push publishes as it packages. Fixes #1 --- .github/workflows/build-linux.yml | 16 ++ .github/workflows/build-windows.yml | 13 ++ core/addon/binding.gyp | 16 +- core/addon/package.json | 3 +- core/addon/test/self-contained.test.js | 258 +++++++++++++++++++++++++ core/addon/vendor/build-libgit2.sh | 4 + 6 files changed, 308 insertions(+), 2 deletions(-) create mode 100644 core/addon/test/self-contained.test.js diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index 1c62468..bdc640a 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -110,6 +110,22 @@ jobs: working-directory: ui/electron run: npm ci --ignore-scripts + # Compile the addon against Electron's ABI here, before packaging, purely + # so the audit below can gate the release: on a tag push electron-builder + # publishes as it packages, so a check that runs afterwards is too late. + # electron-builder's own @electron/rebuild step then finds it up to date. + - name: Build native addon for Electron + working-directory: ui/electron + run: npx electron-rebuild --only gitbox-addon --force + + # Guards issue #1. The addon must carry libgit2/libssh2/mbedTLS/OpenSSL + # inside itself; when an archive is missing from binding.gyp nothing fails + # at build time — the app just dies on the user's machine with + # "undefined symbol: EVP_des_ede3_cbc". + - name: Verify the native addon is self-contained + working-directory: core/addon + run: npm test + # electron-builder runs @electron/rebuild first (compiling gitbox_addon # and node-pty against Electron via node-gyp + the vendored libs), then # packages the AppImage/deb/rpm/pacman targets declared in package.json. diff --git a/.github/workflows/build-windows.yml b/.github/workflows/build-windows.yml index 41a0e05..a15a28e 100644 --- a/.github/workflows/build-windows.yml +++ b/.github/workflows/build-windows.yml @@ -107,6 +107,19 @@ jobs: working-directory: ui/electron run: npm ci --ignore-scripts + # Built here (not left to electron-builder) so the audit below runs before + # a tag push publishes the installers. See build-linux.yml for the why. + - name: Build native addon for Electron + working-directory: ui/electron + run: npx electron-rebuild --only gitbox-addon --force + + # Guards issue #1. On Windows this checks the addon imports no DLL beyond + # the system set (dumpbin comes from the MSVC environment above); the + # symbol-level audit needs nm and skips itself here. + - name: Verify the native addon is self-contained + working-directory: core/addon + run: npm test + # electron-builder runs @electron/rebuild first (compiling gitbox_addon # and node-pty against Electron via node-gyp + the vendored libs), then # packages the NSIS and MSI targets declared in package.json. diff --git a/core/addon/binding.gyp b/core/addon/binding.gyp index 8e3f5af..fe0f9e6 100644 --- a/core/addon/binding.gyp +++ b/core/addon/binding.gyp @@ -24,13 +24,25 @@ }, "conditions": [ ["OS=='linux'", { + # Archive order matters: libssh2 pulls symbols out of libcrypto, so + # libcrypto.a must come after it. Linking the vendored static + # libcrypto is NOT optional — libssh2's crypt-method table holds + # direct data relocations to EVP_* (R_X86_64_64), which the loader + # must resolve at dlopen time. Leave it out and the addon silently + # borrows the host process's OpenSSL: plain node exports its own + # (so it "works"), but Electron ships BoringSSL and has no + # EVP_des_ede3_cbc, so the app dies at startup with + # "undefined symbol: EVP_des_ede3_cbc" (issue #1). "libraries": [ "<(module_root_dir)/vendor/libgit2/install/lib/libgit2.a", "<(module_root_dir)/vendor/libssh2/install/lib/libssh2.a", + "<(module_root_dir)/vendor/openssl/install/lib/libcrypto.a", "<(module_root_dir)/vendor/mbedtls/install/lib/libmbedtls.a", "<(module_root_dir)/vendor/mbedtls/install/lib/libmbedx509.a", "<(module_root_dir)/vendor/mbedtls/install/lib/libmbedcrypto.a", - "-lrt" + "-lrt", + "-ldl", + "-lpthread" ], "ldflags": [ "-Wl,-Bsymbolic", @@ -38,9 +50,11 @@ ] }], ["OS=='mac'", { + # Same libssh2 -> libcrypto rule as Linux; see the note above. "libraries": [ "<(module_root_dir)/vendor/libgit2/install/lib/libgit2.a", "<(module_root_dir)/vendor/libssh2/install/lib/libssh2.a", + "<(module_root_dir)/vendor/openssl/install/lib/libcrypto.a", "<(module_root_dir)/vendor/mbedtls/install/lib/libmbedtls.a", "<(module_root_dir)/vendor/mbedtls/install/lib/libmbedx509.a", "<(module_root_dir)/vendor/mbedtls/install/lib/libmbedcrypto.a" diff --git a/core/addon/package.json b/core/addon/package.json index 0eb3c6b..367a2f8 100644 --- a/core/addon/package.json +++ b/core/addon/package.json @@ -5,7 +5,8 @@ "main": "index.js", "gypfile": true, "scripts": { - "build": "node-gyp rebuild" + "build": "node-gyp rebuild", + "test": "node --test test/*.test.js" }, "dependencies": { "bindings": "^1.5.0", diff --git a/core/addon/test/self-contained.test.js b/core/addon/test/self-contained.test.js new file mode 100644 index 0000000..b97c19f --- /dev/null +++ b/core/addon/test/self-contained.test.js @@ -0,0 +1,258 @@ +'use strict'; + +// Regression tests for the promise that makes GitBox portable: gitbox_addon.node +// must carry libgit2, libssh2, mbedTLS and OpenSSL *inside* itself and import +// nothing from the host beyond the C/C++ runtime and Node's own N-API. +// +// This is what broke in issue #1. binding.gyp linked libssh2.a but not the +// vendored libcrypto.a, so libssh2's crypt-method table kept unresolved data +// relocations (R_X86_64_64) to EVP_des_ede3_cbc & friends. Nothing failed at +// build time, and nothing failed under plain `node` either — node exports its +// own statically linked OpenSSL, so the addon quietly borrowed those symbols. +// Electron ships BoringSSL and exports no EVP_des_ede3_cbc, so the packaged app +// died at startup on any machine that did not happen to have a libcrypto +// already mapped into the process: +// +// Error: .../gitbox_addon.node: undefined symbol: EVP_des_ede3_cbc +// +// Because "does it load?" depends on what the host has lying around, the +// authoritative test here is the static import audit, not a runtime load. +// +// Run with: npm test (from core/addon, after building the addon) + +const test = require('node:test'); +const assert = require('node:assert'); +const fs = require('node:fs'); +const path = require('node:path'); +const { spawnSync } = require('node:child_process'); + +// GITBOX_ADDON_PATH lets this audit run against any build of the addon (a +// packaged one, or a deliberately broken one to prove the audit still bites). +const ADDON = process.env.GITBOX_ADDON_PATH + ? path.resolve(process.env.GITBOX_ADDON_PATH) + : path.join(__dirname, '..', 'build', 'Release', 'gitbox_addon.node'); + +// Symbols that must never be imported: they belong to the libraries we vendor +// and statically link, so seeing one as *undefined* means an archive is missing +// from binding.gyp and the host is being asked to supply it. +const VENDORED_SYMBOLS = [ + // OpenSSL / BoringSSL surface (libssh2's crypto backend) + /^(EVP|SSL|BIO|ERR|RSA|DSA|DH|EC|ECDSA|ECDH|BN|X509|ASN1|PEM|HMAC|CMAC|OPENSSL|OSSL|CRYPTO|RAND|OBJ|NCONF|CONF|UI|ENGINE|PKCS|SHA1|SHA224|SHA256|SHA384|SHA512|MD5|AES|DES|d2i|i2d)_/, + /^(libssh2|git|mbedtls|psa)_/, // libssh2, libgit2, mbedTLS + /^(deflate|inflate|compress|uncompress|crc32|adler32|zlib)/, // bundled zlib +]; + +const ALLOWED_DEPS = { + linux: [ + /^libc\.so\.\d+$/, + /^libm\.so\.\d+$/, + /^libstdc\+\+\.so\.\d+$/, + /^libgcc_s\.so\.\d+$/, + /^libdl\.so\.\d+$/, + /^libpthread\.so\.\d+$/, + /^librt\.so\.\d+$/, + /^ld-linux.*\.so.*$/, + ], + darwin: [/^\/usr\/lib\//, /^\/System\/Library\//], + win32: [ + /^(KERNEL32|USER32|ADVAPI32|CRYPT32|WS2_32|RPCRT4|SECUR32|OLE32|BCRYPT|SHELL32|SHLWAPI|OLEAUT32|VERSION|DBGHELP|WINMM|PSAPI|USERENV|IPHLPAPI|api-ms-win-.*|VCRUNTIME.*|ucrtbase|MSVCP.*|node)\.(dll|DLL|exe|EXE)$/i, + ], +}; + +function tool(name) { + const probe = spawnSync(process.platform === 'win32' ? 'where' : 'which', [name]); + return probe.status === 0; +} + +function run(cmd, args) { + const out = spawnSync(cmd, args, { encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 }); + if (out.status !== 0) throw new Error(`${cmd} ${args.join(' ')} failed: ${out.stderr}`); + return out.stdout; +} + +// Undefined (imported) dynamic symbols, version suffix stripped. +function importedSymbols(file) { + const flag = process.platform === 'darwin' ? '-u' : '-D --undefined-only'; + const out = run('nm', [...flag.split(' '), file]); + return [ + ...new Set( + out + .split('\n') + .map((line) => { + const m = line.match(/^\s*(?:[0-9a-f]*\s+)?U\s+(\S+)$/) || line.match(/^\s*(_?\w[\w.$]*)$/); + return m ? m[1].replace(/@.*$/, '') : null; + }) + .filter(Boolean) + // Mach-O prefixes every C symbol with an underscore. + .map((s) => (process.platform === 'darwin' ? s.replace(/^_/, '') : s)), + ), + ]; +} + +// Symbols the file defines itself (static symbol table, not just exports: the +// linker flags hide the vendored ones from .dynsym on purpose). +function definedSymbols(file) { + const out = run('nm', ['--defined-only', file]); + return new Set( + out + .split('\n') + .map((line) => line.trim().split(/\s+/).pop()) + .filter(Boolean) + .map((s) => (process.platform === 'darwin' ? s.replace(/^_/, '') : s)), + ); +} + +function dynamicDeps(file) { + if (process.platform === 'linux') { + return run('readelf', ['-d', file]) + .split('\n') + .map((l) => l.match(/\(NEEDED\)\s+Shared library: \[(.+)\]/)) + .filter(Boolean) + .map((m) => m[1]); + } + if (process.platform === 'darwin') { + return run('otool', ['-L', file]) + .split('\n') + .slice(1) + .map((l) => l.trim().split(/\s+/)[0]) + .filter(Boolean); + } + return run('dumpbin', ['/dependents', file]) + .split('\n') + .map((l) => l.trim()) + .filter((l) => /\.dll$/i.test(l)); +} + +// Every symbol the C runtime we link against can supply, so the audit does not +// need a hand-maintained list of libc names. +function runtimeExports(file) { + const exports = new Set(); + for (const line of run('ldd', [file]).split('\n')) { + const m = line.match(/=>\s*(\/\S+)/); + if (!m) continue; + for (const sym of run('nm', ['-D', '--defined-only', m[1]]).split('\n')) { + const name = sym.trim().split(/\s+/).pop(); + if (name) exports.add(name.replace(/@.*$/, '')); + } + } + return exports; +} + +const EXPORTS = ['status', 'log', 'branches', 'fetch', 'pull', 'push', 'clone', 'mergeBranch']; + +const built = fs.existsSync(ADDON); +const canInspect = tool('nm') && (process.platform !== 'linux' || tool('readelf')); + +test('native addon self-containment', { skip: built ? false : `${ADDON} not built — run npm run build first` }, (t) => { + t.test('imports nothing from the vendored crypto/git libraries (issue #1)', (t) => { + if (!canInspect) return t.skip('nm/readelf unavailable on this host'); + + const leaked = importedSymbols(ADDON).filter((s) => VENDORED_SYMBOLS.some((re) => re.test(s))); + + assert.deepStrictEqual( + leaked, + [], + `The addon expects the host to provide ${leaked.length} symbol(s) that should be ` + + `statically linked into it: ${leaked.slice(0, 12).join(', ')}. This is issue #1: an ` + + `archive is missing from binding.gyp's libraries list (libssh2 needs the vendored ` + + `libcrypto.a after it), so the app dies at startup under Electron with ` + + `"undefined symbol: ${leaked[0]}".`, + ); + }); + + t.test('every imported symbol resolves from the C runtime or N-API', (t) => { + if (process.platform !== 'linux' || !canInspect || !tool('ldd')) { + return t.skip('ldd-based resolution check runs on Linux only'); + } + + const available = runtimeExports(ADDON); + const unresolved = importedSymbols(ADDON).filter( + (s) => !available.has(s) && !/^(napi_|node_api_)/.test(s), + ); + + assert.deepStrictEqual( + unresolved, + [], + `Imported symbols that no linked library provides: ${unresolved.join(', ')}. ` + + `They would only resolve if the host process happened to have a matching ` + + `library mapped in — exactly the fragility behind issue #1.`, + ); + }); + + t.test('links no shared library beyond the C/C++ runtime', (t) => { + const allowed = ALLOWED_DEPS[process.platform]; + if (!allowed) return t.skip(`no dependency allowlist for ${process.platform}`); + if (process.platform === 'win32' && !tool('dumpbin')) return t.skip('dumpbin unavailable'); + + const unexpected = dynamicDeps(ADDON).filter((dep) => !allowed.some((re) => re.test(dep))); + + assert.deepStrictEqual( + unexpected, + [], + `The addon must not depend on system libraries: ${unexpected.join(', ')}. ` + + `GitBox ships no git/libgit2/OpenSSL runtime, so anything here breaks the ` + + `AppImage and every distro whose version differs.`, + ); + }); + + t.test('statically contains libgit2, libssh2, mbedTLS and OpenSSL', (t) => { + if (!canInspect) return t.skip('nm unavailable on this host'); + if (process.platform === 'win32') return t.skip('SSH/OpenSSL are not enabled on Windows yet'); + + const defined = definedSymbols(ADDON); + // One representative symbol per vendored library. EVP_des_ede3_cbc is + // the one issue #1 reported missing, so it is the canary here. + for (const symbol of [ + 'git_libgit2_init', + 'libssh2_session_init_ex', + 'mbedtls_ssl_init', + 'EVP_des_ede3_cbc', + ]) { + assert.ok( + defined.has(symbol), + `${symbol} is not compiled into the addon — its library was dropped from ` + + `binding.gyp or built without the feature.`, + ); + } + }); + + // Node-API keeps the addon ABI-stable, so this loads under any node or + // Electron version — the point is that it loads at all. + t.test('loads and exposes the native git API', () => { + const addon = require(ADDON); + for (const fn of EXPORTS) assert.strictEqual(typeof addon[fn], 'function', `missing export: ${fn}`); + }); + + // The runtime that actually broke in issue #1. Weaker than the audit above + // (a host with a compatible libcrypto already mapped into the process can + // mask a missing archive), but it is the exact failing path users hit. + t.test('loads inside Electron, the runtime that ships BoringSSL', (t) => { + const electron = electronBinary(); + if (!electron) return t.skip('no electron binary installed under ui/electron'); + + const probe = spawnSync( + electron, + ['-e', `console.log(Object.keys(require(${JSON.stringify(ADDON)})).join(','))`], + { encoding: 'utf8', env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' } }, + ); + + assert.strictEqual( + probe.status, + 0, + `Electron could not load the addon: ${(probe.stderr || '').trim()}`, + ); + const keys = probe.stdout.trim().split('\n').pop().split(','); + for (const fn of EXPORTS) assert.ok(keys.includes(fn), `missing export: ${fn}`); + }); +}); + +function electronBinary() { + try { + const pkg = path.join(__dirname, '..', '..', '..', 'ui', 'electron', 'node_modules', 'electron'); + const exe = fs.readFileSync(path.join(pkg, 'path.txt'), 'utf8').trim(); + const full = path.join(pkg, 'dist', exe); + return fs.existsSync(full) ? full : null; + } catch { + return null; + } +} diff --git a/core/addon/vendor/build-libgit2.sh b/core/addon/vendor/build-libgit2.sh index 993d473..bdaa5ed 100755 --- a/core/addon/vendor/build-libgit2.sh +++ b/core/addon/vendor/build-libgit2.sh @@ -69,6 +69,10 @@ if [ -d "$MBED_PREFIX/lib64" ] && [ ! -e "$MBED_PREFIX/lib" ]; then ln -s lib64 # OpenSSH 7.8+ would simply be rejected. Verified against a real server: ECDSA # authenticated, RSA-in-OpenSSH-format did not. OpenSSL covers every key type # users actually have, so it is built statically here and linked into libssh2. +# +# NOTE: libcrypto.a must ALSO be listed in binding.gyp after libssh2.a. Linking +# libssh2 alone leaves its EVP_* references to the host process (issue #1), and +# Electron's BoringSSL does not provide them. echo ">> OpenSSL $OPENSSL_TAG -> $SSL_PREFIX" if [ ! -f "$SSL_SRC/Configure" ]; then rm -rf "$SSL_SRC" From 760d67365d528554773bc629543206a5bff6620c Mon Sep 17 00:00:00 2001 From: Gustavo Will <48598429+gitgusilva@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:48:40 -0300 Subject: [PATCH 3/5] fix(addon): make the self-containment audit run on node 20 The audit failed on CI for two reasons of its own, neither of them a problem with the addon. Nesting the checks as subtests of one parent test only works on node 22+. node 20, which CI runs, does not await subtests declared inside a synchronous parent callback: it cancels them with "test did not finish before its parent", so five of six checks never ran there while all six passed locally. Declare them flat. The resolution check also read ldd output for "=>" lines only, missing the dynamic loader's own entry, which has no "=>". The loader is what exports __tls_get_addr, and the ubuntu-22.04 toolchain emits a reference to it, so a legitimate import was reported as unresolvable. Verified on node 20 and node 24: six passes against the fixed addon, three failures against a deliberately broken one. --- core/addon/test/self-contained.test.js | 184 +++++++++++++------------ 1 file changed, 96 insertions(+), 88 deletions(-) diff --git a/core/addon/test/self-contained.test.js b/core/addon/test/self-contained.test.js index b97c19f..c1f17e2 100644 --- a/core/addon/test/self-contained.test.js +++ b/core/addon/test/self-contained.test.js @@ -128,7 +128,10 @@ function dynamicDeps(file) { function runtimeExports(file) { const exports = new Set(); for (const line of run('ldd', [file]).split('\n')) { - const m = line.match(/=>\s*(\/\S+)/); + // "libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x…)" plus the dynamic + // loader's own "/lib64/ld-linux-x86-64.so.2 (0x…)" line, which has no + // "=>" — miss it and __tls_get_addr looks like an unresolved import. + const m = line.match(/=>\s*(\/\S+)/) || line.match(/^\s*(\/\S+)\s+\(0x/); if (!m) continue; for (const sym of run('nm', ['-D', '--defined-only', m[1]]).split('\n')) { const name = sym.trim().split(/\s+/).pop(); @@ -143,107 +146,112 @@ const EXPORTS = ['status', 'log', 'branches', 'fetch', 'pull', 'push', 'clone', const built = fs.existsSync(ADDON); const canInspect = tool('nm') && (process.platform !== 'linux' || tool('readelf')); -test('native addon self-containment', { skip: built ? false : `${ADDON} not built — run npm run build first` }, (t) => { - t.test('imports nothing from the vendored crypto/git libraries (issue #1)', (t) => { - if (!canInspect) return t.skip('nm/readelf unavailable on this host'); +const skip = built ? false : `${ADDON} not built — run "npm run build" first`; - const leaked = importedSymbols(ADDON).filter((s) => VENDORED_SYMBOLS.some((re) => re.test(s))); +// Keep these checks flat. node 20 (what CI runs) does not await subtests +// declared inside a synchronous parent test: it cancels them and reports +// "test did not finish before its parent", so a nested suite passes locally on +// node 22+ while quietly running one of six checks on CI. - assert.deepStrictEqual( - leaked, - [], - `The addon expects the host to provide ${leaked.length} symbol(s) that should be ` + - `statically linked into it: ${leaked.slice(0, 12).join(', ')}. This is issue #1: an ` + - `archive is missing from binding.gyp's libraries list (libssh2 needs the vendored ` + - `libcrypto.a after it), so the app dies at startup under Electron with ` + - `"undefined symbol: ${leaked[0]}".`, - ); - }); - - t.test('every imported symbol resolves from the C runtime or N-API', (t) => { - if (process.platform !== 'linux' || !canInspect || !tool('ldd')) { - return t.skip('ldd-based resolution check runs on Linux only'); - } +test('imports nothing from the vendored crypto/git libraries (issue #1)', { skip }, (t) => { + if (!canInspect) return t.skip('nm/readelf unavailable on this host'); - const available = runtimeExports(ADDON); - const unresolved = importedSymbols(ADDON).filter( - (s) => !available.has(s) && !/^(napi_|node_api_)/.test(s), - ); + const leaked = importedSymbols(ADDON).filter((s) => VENDORED_SYMBOLS.some((re) => re.test(s))); - assert.deepStrictEqual( - unresolved, - [], - `Imported symbols that no linked library provides: ${unresolved.join(', ')}. ` + - `They would only resolve if the host process happened to have a matching ` + - `library mapped in — exactly the fragility behind issue #1.`, - ); - }); + assert.deepStrictEqual( + leaked, + [], + `The addon expects the host to provide ${leaked.length} symbol(s) that should be ` + + `statically linked into it: ${leaked.slice(0, 12).join(', ')}. This is issue #1: an ` + + `archive is missing from binding.gyp's libraries list (libssh2 needs the vendored ` + + `libcrypto.a after it), so the app dies at startup under Electron with ` + + `"undefined symbol: ${leaked[0]}".`, + ); +}); - t.test('links no shared library beyond the C/C++ runtime', (t) => { - const allowed = ALLOWED_DEPS[process.platform]; - if (!allowed) return t.skip(`no dependency allowlist for ${process.platform}`); - if (process.platform === 'win32' && !tool('dumpbin')) return t.skip('dumpbin unavailable'); +test('every imported symbol resolves from the C runtime or N-API', { skip }, (t) => { + if (process.platform !== 'linux' || !canInspect || !tool('ldd')) { + return t.skip('ldd-based resolution check runs on Linux only'); + } - const unexpected = dynamicDeps(ADDON).filter((dep) => !allowed.some((re) => re.test(dep))); + const available = runtimeExports(ADDON); + const unresolved = importedSymbols(ADDON).filter( + (s) => !available.has(s) && !/^(napi_|node_api_)/.test(s), + ); - assert.deepStrictEqual( - unexpected, - [], - `The addon must not depend on system libraries: ${unexpected.join(', ')}. ` + - `GitBox ships no git/libgit2/OpenSSL runtime, so anything here breaks the ` + - `AppImage and every distro whose version differs.`, - ); - }); + assert.deepStrictEqual( + unresolved, + [], + `Imported symbols that no linked library provides: ${unresolved.join(', ')}. ` + + `They would only resolve if the host process happened to have a matching ` + + `library mapped in — exactly the fragility behind issue #1.`, + ); +}); - t.test('statically contains libgit2, libssh2, mbedTLS and OpenSSL', (t) => { - if (!canInspect) return t.skip('nm unavailable on this host'); - if (process.platform === 'win32') return t.skip('SSH/OpenSSL are not enabled on Windows yet'); +test('links no shared library beyond the C/C++ runtime', { skip }, (t) => { + const allowed = ALLOWED_DEPS[process.platform]; + if (!allowed) return t.skip(`no dependency allowlist for ${process.platform}`); + if (process.platform === 'win32' && !tool('dumpbin')) return t.skip('dumpbin unavailable'); - const defined = definedSymbols(ADDON); - // One representative symbol per vendored library. EVP_des_ede3_cbc is - // the one issue #1 reported missing, so it is the canary here. - for (const symbol of [ - 'git_libgit2_init', - 'libssh2_session_init_ex', - 'mbedtls_ssl_init', - 'EVP_des_ede3_cbc', - ]) { - assert.ok( - defined.has(symbol), - `${symbol} is not compiled into the addon — its library was dropped from ` + - `binding.gyp or built without the feature.`, - ); - } - }); + const unexpected = dynamicDeps(ADDON).filter((dep) => !allowed.some((re) => re.test(dep))); - // Node-API keeps the addon ABI-stable, so this loads under any node or - // Electron version — the point is that it loads at all. - t.test('loads and exposes the native git API', () => { - const addon = require(ADDON); - for (const fn of EXPORTS) assert.strictEqual(typeof addon[fn], 'function', `missing export: ${fn}`); - }); + assert.deepStrictEqual( + unexpected, + [], + `The addon must not depend on system libraries: ${unexpected.join(', ')}. ` + + `GitBox ships no git/libgit2/OpenSSL runtime, so anything here breaks the ` + + `AppImage and every distro whose version differs.`, + ); +}); - // The runtime that actually broke in issue #1. Weaker than the audit above - // (a host with a compatible libcrypto already mapped into the process can - // mask a missing archive), but it is the exact failing path users hit. - t.test('loads inside Electron, the runtime that ships BoringSSL', (t) => { - const electron = electronBinary(); - if (!electron) return t.skip('no electron binary installed under ui/electron'); +test('statically contains libgit2, libssh2, mbedTLS and OpenSSL', { skip }, (t) => { + if (!canInspect) return t.skip('nm unavailable on this host'); + if (process.platform === 'win32') return t.skip('SSH/OpenSSL are not enabled on Windows yet'); - const probe = spawnSync( - electron, - ['-e', `console.log(Object.keys(require(${JSON.stringify(ADDON)})).join(','))`], - { encoding: 'utf8', env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' } }, + const defined = definedSymbols(ADDON); + // One representative symbol per vendored library. EVP_des_ede3_cbc is + // the one issue #1 reported missing, so it is the canary here. + for (const symbol of [ + 'git_libgit2_init', + 'libssh2_session_init_ex', + 'mbedtls_ssl_init', + 'EVP_des_ede3_cbc', + ]) { + assert.ok( + defined.has(symbol), + `${symbol} is not compiled into the addon — its library was dropped from ` + + `binding.gyp or built without the feature.`, ); + } +}); - assert.strictEqual( - probe.status, - 0, - `Electron could not load the addon: ${(probe.stderr || '').trim()}`, - ); - const keys = probe.stdout.trim().split('\n').pop().split(','); - for (const fn of EXPORTS) assert.ok(keys.includes(fn), `missing export: ${fn}`); - }); +// Node-API keeps the addon ABI-stable, so this loads under any node or Electron +// version — the point is that it loads at all. +test('loads and exposes the native git API', { skip }, () => { + const addon = require(ADDON); + for (const fn of EXPORTS) assert.strictEqual(typeof addon[fn], 'function', `missing export: ${fn}`); +}); + +// The runtime that actually broke in issue #1. Weaker than the audits above (a +// host with a compatible libcrypto already mapped into the process can mask a +// missing archive), but it is the exact failing path users hit. +test('loads inside Electron, the runtime that ships BoringSSL', { skip }, (t) => { + const electron = electronBinary(); + if (!electron) return t.skip('no electron binary installed under ui/electron'); + + const probe = spawnSync( + electron, + ['-e', `console.log(Object.keys(require(${JSON.stringify(ADDON)})).join(','))`], + { encoding: 'utf8', env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' } }, + ); + + assert.strictEqual( + probe.status, + 0, + `Electron could not load the addon: ${(probe.stderr || '').trim()}`, + ); + const keys = probe.stdout.trim().split('\n').pop().split(','); + for (const fn of EXPORTS) assert.ok(keys.includes(fn), `missing export: ${fn}`); }); function electronBinary() { From a0b829d2f1f5b1fd9528d1d1d54e4e7f2091ada4 Mon Sep 17 00:00:00 2001 From: Gustavo Will <48598429+gitgusilva@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:57:44 -0300 Subject: [PATCH 4/5] fix(addon): run the audit with a path cmd.exe can handle npm test used "node --test test/*.test.js", which the Windows shell does not expand, so the job died with "Could not find D:\a\gitbox\gitbox\core\addon\test\*.test.js". Name the file explicitly. It is the only form that works on every target: `--test ` runs on node 20 but not 24, `--test ` runs on 24 but not 20, and only a plain path needs no shell expansion. Verified with npm test on both node versions. --- core/addon/package.json | 2 +- core/addon/test/self-contained.test.js | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/core/addon/package.json b/core/addon/package.json index 367a2f8..a70ee0d 100644 --- a/core/addon/package.json +++ b/core/addon/package.json @@ -6,7 +6,7 @@ "gypfile": true, "scripts": { "build": "node-gyp rebuild", - "test": "node --test test/*.test.js" + "test": "node --test test/self-contained.test.js" }, "dependencies": { "bindings": "^1.5.0", diff --git a/core/addon/test/self-contained.test.js b/core/addon/test/self-contained.test.js index c1f17e2..cbc00be 100644 --- a/core/addon/test/self-contained.test.js +++ b/core/addon/test/self-contained.test.js @@ -19,6 +19,11 @@ // authoritative test here is the static import audit, not a runtime load. // // Run with: npm test (from core/addon, after building the addon) +// +// That script names this file explicitly. It is the only form that works +// everywhere: cmd.exe does not expand "test/*.test.js", `--test ` works on +// node 20 but not 24, and `--test ` works on 24 but not 20. New test +// files have to be added to the script by hand. const test = require('node:test'); const assert = require('node:assert'); From d76585bf80dbc7c5449ccc962b3c12a09c643c45 Mon Sep 17 00:00:00 2001 From: Gustavo Will <48598429+gitgusilva@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:22:56 -0300 Subject: [PATCH 5/5] ci: exempt docs from the build instead of allow-listing code, and cancel superseded runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The paths filter was an allow-list (core, app, ui, domain, shared), which fails in the wrong direction: a new code directory nobody remembers to add gets no CI at all, silently. Invert it — build by default, exempt only what is provably documentation. The required-status-check caveat is unchanged and now written down: any filter means some PRs produce no run, and a required check that never runs pins the PR to "Expected — waiting for status". Gating the expensive steps instead of the trigger is the way to have both. Also add a concurrency group so a new push to a PR cancels the run it supersedes; three runs piled up in parallel on this branch alone. Limit cancel-in-progress to pull requests: a tag build ends in `electron-builder --publish always`, and killing that mid-upload can leave a release whose latest-linux.yml points at assets that never finished uploading, which is what electron-updater then serves to every client. --- .github/workflows/build-linux.yml | 38 ++++++++++++++++++++--------- .github/workflows/build-windows.yml | 21 ++++++++++------ 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index bdc640a..150af68 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -15,20 +15,36 @@ on: tags: - "v*" # Pull requests build and verify but never publish (the --publish flag below - # only says "always" for a tag). The paths filter keeps doc-only PRs from - # spending ~20 min packaging four installers; note that a required status - # check on this workflow would block those PRs forever, so if you make it - # required, drop the filter. + # only says "always" for a tag). + # + # paths-ignore, not paths: the default is to build, and only what is provably + # documentation is exempt. An allow-list would silently skip CI for any new + # code directory nobody remembered to add. + # + # This still means some PRs get no run at all, so do NOT mark this workflow as + # a required status check — a required check that never runs leaves the PR + # stuck on "Expected — waiting for status" forever. To have both, drop the + # filter and gate the expensive steps with an `if:` on the changed paths + # instead, so the check always reports a conclusion. pull_request: branches: - main - paths: - - "core/**" - - "app/**" - - "ui/**" - - "domain/**" - - "shared/**" - - ".github/workflows/build-linux.yml" + paths-ignore: + - "**.md" + - "site/**" + - "images/**" + - "LICENSE" + +# One build per ref: a new push to a PR supersedes the one still running. +# +# cancel-in-progress is deliberately limited to pull requests. A tag build ends +# in `electron-builder --publish always`, and killing that mid-upload can leave +# the GitHub Release with a latest-linux.yml pointing at assets that were never +# finished uploading — which is exactly what electron-updater then hands to +# every installed client. +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} # Needed for electron-builder to upload artifacts to the GitHub Release on a # tag push. The default GITHUB_TOKEN is read-only, so publishing fails with diff --git a/.github/workflows/build-windows.yml b/.github/workflows/build-windows.yml index a15a28e..c32cf7f 100644 --- a/.github/workflows/build-windows.yml +++ b/.github/workflows/build-windows.yml @@ -15,17 +15,22 @@ on: push: tags: - "v*" - # Same rules as build-linux.yml: PRs build and verify, never publish. + # Same rules as build-linux.yml: PRs build and verify, never publish, and the + # filter exempts documentation rather than allow-listing code. pull_request: branches: - main - paths: - - "core/**" - - "app/**" - - "ui/**" - - "domain/**" - - "shared/**" - - ".github/workflows/build-windows.yml" + paths-ignore: + - "**.md" + - "site/**" + - "images/**" + - "LICENSE" + +# One build per ref, cancelling superseded PR runs only — never a publishing tag +# build. See build-linux.yml for why. +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} # Needed for electron-builder to upload artifacts to the GitHub Release on a # tag push. The default GITHUB_TOKEN is read-only, so publishing fails with