Skip to content

Fix macOS signing key leakage in packaged apps - #390

Merged
juanmaguitar merged 3 commits into
trunkfrom
iangmaia/prevent-signing-key-packaging
Aug 20, 2026
Merged

juanmaguitar merged 3 commits into
trunkfrom
iangmaia/prevent-signing-key-packaging

Conversation

@iangmaia

@iangmaia iangmaia commented Aug 20, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes AINFRA-2941

Why

macOS signing wrote the App Store Connect API key to .codesigning/ in the project root. Because electron-builder included that directory in app.asar, packaged builds contained the key.

What changes

  • Write the notarization key to a private temporary directory outside the project and clean it up when the build exits or is cancelled.
  • Fail immediately if the temporary directory or key cannot be created.
  • Exclude .codesigning from packages and use a CI canary to verify the full packaged payload.

How to test this

Starting state: This PR's GitHub Actions and Buildkite builds correspond to the current head commit.

  1. Confirm the packaged smoke jobs pass on macOS and Windows; the staged .codesigning/apple_api_key canary must be absent from each package.
  2. Confirm the Buildkite macOS signing, packaging, notarization, and verify_code_signing steps pass.
  3. Mount the signed DMG and launch the app; it must open normally with a valid signature.

What must not have happened: No .codesigning directory or apple_api_key may be present in a packaged app, and the temporary key must not survive normal exit, setup failure, or SIGTERM cancellation.

Platforms: macOS for signing; macOS and Windows for packaged-payload coverage.

Risks and limitations

There is no visible UI change. SIGTERM cleanup is covered by a process-level test; SIGKILL cannot run shell cleanup traps.


Review outcome (required — see AGENTS.md)

0 [fix here] · 0 [follow-up]. No findings across the five review dimensions.

@iangmaia iangmaia self-assigned this Aug 20, 2026
@iangmaia
iangmaia requested review from juanmaguitar and a balanced review from Copilot August 20, 2026 16:15
@iangmaia
iangmaia marked this pull request as ready for review August 20, 2026 16:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Moves macOS notarization credentials outside the project and adds defense-in-depth packaging checks.

Changes:

  • Creates and cleans up the API key in a private temporary directory.
  • Excludes .codesigning from packaged applications.
  • Adds unit and packaged-payload regression coverage.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
.buildkite/commands/setup_macos_code_signing.sh Secures temporary key handling and cleanup.
.github/workflows/e2e.yml Stages a packaging-exclusion canary.
package.json Excludes .codesigning from packages.
tests/e2e/packaged/smoke.spec.js Checks the complete packaged payload.
tests/unit/macos-signing.test.cjs Tests creation failures and cleanup paths.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@iangmaia
iangmaia marked this pull request as draft August 20, 2026 16:20
@iangmaia
iangmaia marked this pull request as ready for review August 20, 2026 16:26
@iangmaia iangmaia added the bug Something isn't working label Aug 20, 2026
@iangmaia
iangmaia requested a review from mokagio August 20, 2026 19:06
@juanmaguitar
juanmaguitar merged commit a0fcbc9 into trunk Aug 20, 2026
11 checks passed
@juanmaguitar
juanmaguitar deleted the iangmaia/prevent-signing-key-packaging branch August 20, 2026 20:00
juanmaguitar pushed a commit that referenced this pull request Aug 21, 2026
Cherry-picked from a0fcbc9 onto this stack so no signing build on these
branches writes the rotated App Store Connect key back into the package.
Buildkite stays archived until every open branch carries it.

This branch predates the e2e suite and the test/ -> tests/ move, so the
packaged smoke canary and its workflow hook are left out; the unit test
comes along and keeps its tests/unit/ path, which is what it resolves the
script from.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
iangmaia added a commit that referenced this pull request Aug 21, 2026
## Why

We've rotated the macOS signing credentials, but branches that predate
the secure signing setup (or any new branch accidentally created from an
old revision) could consume them accidentally. This PR creates a new
"credential epoch" by renaming the CI variables so only code with the
safer setup can use the rotated credentials.

## What changes

Buildkite must supply three repository-specific `WPCT_MACOS_SIGNING_*`
inputs, and setup fails with an actionable error if any is missing.

The setup script materializes the private key in a protected temporary
file and exports the `APPLE_API_*` interface required by Electron
Builder. Fastlane explicitly reuses that same interface. CI no longer
injects the legacy names, and repository code no longer references them,
so older code cannot use the rotated credentials.

## How to test this

**Starting state:** Check out this branch on macOS. The protected
signing credentials are not needed for the focused unit tests.

1. Run `node --test tests/unit/macos-signing.test.cjs`. All nine tests
pass, including one rejection case for each missing repository-specific
variable, trace-safety and Fastlane-failure checks, and temporary-key
cleanup coverage.
2. After the new variables are enabled in Buildkite, open the macOS
build for the current head. `npm run dist` and `bundle exec fastlane
verify_code_signing` both pass.

**What must not have happened:** Legacy variables alone must not reach
Fastlane, signing material must not appear in the checkout or packaged
payload, the private key must not appear in shell traces, and the
temporary key must not survive the Buildkite shell.

## Risks and limitations

The new variables live outside this repository and must be configured in
Buildkite before the macOS build can pass. Real signing and notarization
can only be verified through Buildkite.

## Related

Follow-up to #390.

---

<details>
<summary>Design decisions and alternatives considered</summary>

The repository-specific `WPCT_MACOS_SIGNING_*` names create the
credential epoch without needing a version suffix, and routine future
rotations can replace their values without another code change.
`WPCT_MACOS_SIGNING_PRIVATE_KEY` also matches Buildkite's default
`*_PRIVATE_KEY` redaction pattern. Electron Builder requires
`APPLE_API_KEY` to be a filepath, so the key is materialized once and
Fastlane explicitly reuses the same temporary-file-backed interface.

</details>

<details>
<summary>Review outcome (required — see AGENTS.md)</summary>

Final self-review: 0 `[fix here]` · 1 `[follow-up]`. Direct Fastlane
invocation still accepts empty environment entries and remains a
pre-existing follow-up; the normal Buildkite path rejects them. Lint,
all 1,051 tests, all nine focused signing tests, and shell and Ruby
syntax checks pass.

</details>
juanmaguitar added a commit that referenced this pull request Aug 21, 2026
## Why

`v1.0.0` shipped on 14 August. Two of the thirteen commits since then
reach the packaged app:

- [#382](#382) — a
site can be deleted when Git marked it read-only, and the app says so
when it cannot.
- [#390](#390) —
the macOS signing step no longer writes the App Store Connect key into
the project, so it stops being packaged.

The other eleven are tests and documentation.

`electron-builder` derives artifact names from `package.json`, so the
version has to move before the tag and the artifacts exist.

There is a second reason this release is needed rather than optional.
Every macOS artifact published so far carries the notarization key
inside `app.asar` — the incident behind #390. The key is already
revoked, so it is inert, but the download still hands it out, and the
docs site's download button resolves against `releases/latest`.
Publishing v1.0.1 is what stops that button serving the compromised
build; v1.0.0's macOS asset is then deleted.

## What changes

Only the version. All three package-version fields in `package.json` and
`package-lock.json` move together from `1.0.0` to `1.0.1`. No dependency
versions change, and there is no application behaviour change in this
PR.

## How to test this

Platforms: any for the suite; macOS for the artifact check that follows
the merge.

1. `npm run lint` is clean and `npm test` passes.
2. `node -e "console.log(require('./package.json').version)"` prints
`1.0.1`.
3. `python3 -c "import
json;d=json.load(open('package-lock.json'));print(d['version'],
d['packages']['']['version'])"` prints `1.0.1 1.0.1` — both root entries
move together, and no dependency pinned at 1.0.0 was touched.

**What must not have happened:** no dependency version may change. `git
diff` must show exactly three lines, all of them the root package
version.

After merge, the release build must produce artifacts named
`wordpress-contributor-toolkit-1.0.1-*`, and the macOS one must contain
no `.codesigning` directory and no `apple_api_key`.

## Risks and limitations

No UI change. The risk is a mis-scoped edit in `package-lock.json`,
where several dependencies also sit at version `1.0.0`; the replacement
was confined to the first 2000 bytes of the file, which covers the two
root entries and nothing else, and both files were re-parsed as JSON
afterwards.

Verified locally on this branch: lint clean, `npm test` 1046 pass / 0
fail.

---

Review outcome (required — see AGENTS.md)

0 `[fix here]` · 0 `[follow-up]`. Deterministic layer only: lint clean,
1046 tests pass, both files valid JSON, diff confined to the three root
version fields. No judgement pass was run — this change has no logic to
review.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
juanmaguitar added a commit that referenced this pull request Sep 11, 2026
…st, Fixes #399, Fixes #387 (#450)

## Why

`build.files` in package.json was an exclusion list, so every top-level
entry of the checkout shipped inside `app.asar` unless someone had named
it. That shape is what packaged the App Store Connect key in #390, and
it is why a developer's checkout ships different contents from a CI
checkout (#387): a baseline `npm run pack:dir` on trunk `4110349` with
seeded leftovers packed `test-results/`, the VitePress build under
`docs/.vitepress/dist/`, and even gitignored personal files
(`CLAUDE.local.md`, `AGENTS.override.md`) next to `tests/`, `docs/`,
`scripts/`, `.buildkite/`, `fastlane/` and every root Markdown file.
This is not a size change and the PR does not claim one: #399 is about
which way the default points.

## What changes

- **`build.files` is now a positive list**: `package.json`, `src/**/*`
(minus `src/renderer/index.jsx`, which the esbuild bundle replaces), and
`local-playground-web/**/*` (optional, served by `src/main.js` when
present). The dugite trims stay as `!` rules; electron-builder applies
only the negative patterns of `files` to `node_modules`, so production
dependencies ship exactly as before (same 28538 entries under
`node_modules`). `!vendor` and `!.codesigning` go, because unlisted is
excluded.
- **Every file under `src/` is checked, not just the archive root.**
`src/**/*` is recursive, so the expected set is derived from Git —
tracked under `src/`, minus the esbuild entry point, plus the two build
outputs that replace it — and compared against a recursive walk inside
the archive. An untracked file anywhere under `src/` fails by name.
Deriving beats enumerating here: a hand-written list of eighty entries
is the exclusion list again.
- **The one native module ships its binary and nothing else, on Windows
too.** The signed Windows artifact carried nineteen MSVC intermediates
beside `fs_ext.node` (`.iobj`, `.ipdb`, `.lib`, `.exp`, the `.vcxproj`
pair, eight `.tlog` logs under `obj/`), about 680 KB that
electron-builder's default excludes miss by an extension. Three `!`
rules trim them. Two assertions hold the line: the build directory in
`app.asar.unpacked` must contain exactly the binary, and the module is
now *loaded* inside the packaged main process rather than only resolved,
so a rule that overreached into the binary, or an ABI mismatch, fails in
CI instead of on a contributor's first site.
- **The bundled npm CLI is now asserted too.** `src/main.js`,
`src/install-runner.js` and `src/script-runner.js` all resolve
`npm/package.json` to run installs and build scripts, and nothing
checked that it survived packaging. It is in `REQUIRED_MODULES` beside
the Playground CLI, the file-lock native module and dugite.
- **The packaged smoke test reads the payload against the same list**,
inside and outside the archive: the root of `app.asar` through
Electron's asar-aware fs from the packaged main process, and the root of
`app.asar.unpacked` on disk. Anything outside `node_modules`,
`package.json`, `src` (and the optional `local-playground-web`) fails,
and the three required entries must be present. The in-asar
`.codesigning` check is subsumed; the plain-fs walk of the whole payload
for `.codesigning` stays, since it covers places neither root listing
reaches.
- **CI seeds the leftovers before packaging**: the packaged job stages a
signing key, `test-results/`, a top-level `dist/intermediate/` and
`docs/.vitepress/dist/` and then proves none ships, on macOS and
Windows.
- **Root cause of the "nested dist" in #387, confirmed before fixing**:
electron-builder always excludes its own output directory
(`directories.output`, so a top-level `dist/` never shipped), but
nothing covered a `dist/` anywhere else. The one in the asar is
`docs/.vitepress/dist/`, the VitePress build. `test-results/` had no
rule at all.

Not in this PR: `build/icon.png`, which `src/main.js` references on
Linux only. electron-builder excludes `buildResources` by default, so
that path has never resolved in a packaged app; the allow-list does not
change it, and Linux is outside the v1.1.0 validation scope (#388).

## How to test this

Platforms: macOS and Windows, because the assertion is about the
packaged payload and the payload is built per platform.

**Automated, any platform with a checkout:**

In the repository root:

```
npm run build:once && CSC_IDENTITY_AUTO_DISCOVERY=false npm run pack:dir
npm run test:e2e:packaged
```

Fifteen tests pass. To watch the new assertion fail on the old rule:
check out trunk's package.json (`git show trunk:package.json >
package.json`), rebuild with the same two commands, and run `npx
--no-install playwright test --project=packaged -g allow-listed`. It
fails listing every root entry that used to ship (verified on macOS: 27
offenders, `test-results` and `docs` among them).

**Automated, run in CI at `010ec0e`:** the packaged job seeds all four
leftovers and then packages, on both platforms. Thirteen tests passed on
**macOS** (job `103260902735`) and thirteen on **Windows** (job
`103260902938`), all seven checks green. **At `7a40ce8`:** fifteen pass
on **macOS** (job `103300828500`) and fifteen on **Windows** (job
`103300828243`). The Windows run is the one that proves the
native-module trim: it is the platform that leaves the MSVC working set
behind, and its build directory now holds exactly `fs_ext.node`. The
module also loads for real on both. Those two runs are the CI half of "a
local build and a CI build contain the same tree".

**Payload verified on the signed Buildkite artifacts, both platforms.**
Buildkite build **#1020** is the build for `7a40ce8`, all three platform
steps green. Its Windows `.exe` was inspected directly, with its SHA-256
(`3c693d13…496c9eb`) matched against the artifact Buildkite lists, so
the file is tied to that exact build and commit. The macOS `.dmg`
inspected is from the branch's previous build (the three earlier commits
are payload-identical to each other; `7a40ce8` changes only the Windows
tree, as the row below shows):

| Check | macOS `.dmg` | Windows `.exe` |
|---|---|---|
| `app.asar` root | `node_modules`, `package.json`, `src` | same |
| `app.asar.unpacked` root | `node_modules` | same |
| `src/` contents vs `git ls-files` | exact match | exact match |
| `tests`, `docs`, `test-results`, `dist`, `scripts`, `fastlane`,
`.buildkite` | absent | absent |
| `.codesigning` anywhere in the payload | absent | absent |
| credential manager and git-lfs in the bundled Git | absent | absent |
| `fs-ext-extra-prebuilt/build/Release` | `fs_ext.node` only |
`fs_ext.node` only (at `7a40ce8`; 19 MSVC intermediates before) |
| Signature | Developer ID Automattic, notarized | Authenticode not
checkable from macOS |

**Same tree, local versus CI (#387's third acceptance): discharged.** A
local `pack:dir` on this branch and the signed macOS artifact list
**28620 identical entries**, the same paths with no diff at all. The
Windows artifact of build #1020 differs from the previous Windows
artifact in exactly the 19 entries `7a40ce8` set out to remove, and in
nothing else. The two platform artifacts now differ only in the bundled
Git (262 entries on macOS, 417 on Windows).

**Manual pass done — JuanMa, Windows 11, Buildkite build #1020 (commit
`7a40ce8`), with Git for Windows installed on the machine, which is the
case that matters.** The installed app booted and painted the site list.
On a site created with the app: two tickets linked (`ticket/60001`,
`ticket/60002`), a switch between them, PR #13456 applied, and an Update
trunk that offered to save the five applied files, then reset the
checkout; afterwards, plain `git status` in the site directory read "On
branch trunk, up to date with origin/trunk, working tree clean" and `git
branch` listed both ticket branches. During a fresh "Create WordPress
Core site", a PowerShell loop over `Get-Process` caught the Git
processes at

```
...\resources\app.asar.unpacked\node_modules\dugite\git\cmd\git.exe
...\resources\app.asar.unpacked\node_modules\dugite\git\mingw64\bin\git.exe
```

and never at the system Git. That is the bundled binary spawning from
the signed installation, with a competing Git on `PATH` and ignored, as
the invariant says.

**Starting state:**

1. Download the macOS `.dmg` (or `.zip`) and the Windows `.exe` from
that build.
2. Install and launch each. The first window paints and the site list
shows "WordPress Core".
3. macOS: in Terminal, from any directory, list the asar root:
`npx @electron/asar list "/Applications/WordPress Contributor
Toolkit.app/Contents/Resources/app.asar" | awk -F/ 'NF>=2{print $2}' |
sort -u`
   Expected: exactly `node_modules`, `package.json`, `src`.
4. Windows: in PowerShell, from any directory (the install folder is
named after the package, not the product):
`npx @electron/asar list
"$env:LOCALAPPDATA\Programs\electron-setup-wordpress-core\resources\app.asar"
| ForEach-Object { ($_ -split '/')[1] } | Sort-Object -Unique`
   Expected: the same three entries.
5. Either platform: create a site or open an existing one and run
**Update trunk** or link a ticket, so the bundled Git spawns from
`app.asar.unpacked`. On **Windows**, also run an `npm install` on a
site, which exercises the bundled npm CLI and the two shims copied out
of `src/`.
6. Same tree, local versus CI (#387's third acceptance). On **macOS**,
in the repository root, build locally and compare the installed
Buildkite artifact against it:

   ```
npm run build:once && CSC_IDENTITY_AUTO_DISCOVERY=false npm run pack:dir
npx @electron/asar list dist/mac-*/*.app/Contents/Resources/app.asar |
sort > /tmp/local.txt
npx @electron/asar list "/Applications/WordPress Contributor
Toolkit.app/Contents/Resources/app.asar" | sort > /tmp/ci.txt
   diff /tmp/local.txt /tmp/ci.txt && echo "same tree"
   ```

Run it from a checkout that has already run the test suites, so the
leftovers are present locally and their absence means something.

**What must not have happened:**

- No `tests`, `docs`, `test-results`, `.codesigning` or any root
Markdown file in the asar listing.
- The bundled Git still works from the installed app (a ticket link or
trunk update completes), which proves the `asarUnpack` rule and the
dugite trims survived the new list.
- The Windows install still runs npm scripts (the `win-spawn-patch.js`
and `electron-node-compat.js` shims are copied out of `src/` at run
time; a dropped `src/` file would surface as a failed `npm install`).

## Risks and limitations

- The two lists (package.json and the spec) are still edited by hand,
deliberately: widening what ships should be a visible edit in two
places. They cannot silently diverge, because a third test derives the
allowed set from `build.files` and asserts it equals the literal list.
- `local-playground-web` is allowed but not asserted present, because it
is not in the repository. A checkout that carries one ships it, as
before.
- Linux artifacts (AppImage, deb, snap) are built with the same list but
not validated here (#388). The Linux window icon path in `src/main.js`
was already unreachable in packaged builds and stays so; recorded here
rather than filed, per the no-new-issues rule for v1.1.0.
- The local-versus-CI tree comparison is a step of the manual pass, not
something CI does on its own: no job has both artifacts. What CI does
prove, on both platforms, is that the four machine-dependent leftovers
are gone and the roots match the list. With a positive list there is
nothing machine-dependent left that could differ, but the diff above is
the evidence rather than the argument.
- **`node_modules` is not governed by the allow-list.** electron-builder
collects production dependencies itself and applies only the `!`
patterns of `files` to them, so a leftover inside a dependency has to be
named. The one such case found, the MSVC working set of
`fs-ext-extra-prebuilt` on Windows, is trimmed in `7a40ce8` and asserted
on both platforms; the packaged smoke test would not have noticed it
before, because the module was resolved but never loaded. Only Windows
can fail the new assertion, so its packaged CI job is the proof, and the
signed Windows artifact of build #1020 was re-inspected: the build
directory holds exactly `fs_ext.node`.
- Code-signing and notarization are unaffected by `files`. The macOS
artifact is signed with Automattic's Developer ID and notarized,
confirmed by `codesign` and `spctl`. The Windows Authenticode signature
could not be checked from macOS.
- electron-builder strips the `build` key from the packaged
`package.json`, so an artifact cannot be tied to a commit from the
inside. The Windows artifact is tied to `7a40ce8` from the outside, by
its SHA-256 against Buildkite build #1020. The macOS `.dmg` inspected
predates `7a40ce8`; that commit changes only the Windows tree, and the
macOS packaged CI job at `7a40ce8` (15/15) covers the rest.

## Related

Fixes #399
Fixes #387
Follow-up to #390 and #396 (the key incident), #377 (the `test/` to
`tests/` move an exclusion list would have had to track).

---

<details>
<summary>Design decisions and alternatives considered</summary>

- **Exact set, not "no offenders only".** The issue asked that the asar
contain no top-level entry outside the list. Requiring the three runtime
entries to be present as well means a broken glob (say `src/**` stops
matching after a rename) fails here rather than at first launch.
- **Adding `!test-results` and `!**/dist` instead** (what #387 first
proposed) was rejected: it keeps the exclusion shape #399 identifies as
the problem, and a `dist/` glob would need care not to strip `dist/`
directories inside published npm packages, which many have.
- **Reading the asar from outside with `@electron/asar`** was rejected
in favour of `readdirSync(app.getAppPath())` inside the packaged
process: it uses the same asar-aware fs the app runs on, needs no extra
dependency in the test, and is the pattern the other in-asar assertions
already use.
- **Seeding in the workflow, not in the test.** The test runs after
packaging, so it cannot seed. The seeds live in the packaged job's
existing canary step. Locally a checkout that has run the suites or the
docs build carries two of the four already, which is what TESTING.md now
says; `.codesigning` exists only on a signing machine, so the complete
proof is the CI run.
- **`build/icon.png` left out.** Adding it would put a new `build` entry
in every asar for a Linux-only path that has never resolved in a
packaged app. Out of scope for v1.1.0.

</details>

<details>
<summary>Review outcome (required — see AGENTS.md)</summary>

**0 [fix here] · 3 [follow-up] — 2 of the 3 follow-ups fixed in the
branch, 1 recorded as a limitation.**

- **Review:** completed — `/self-review`, judgement pass in a fresh
subagent context against
`.github/instructions/code-review.instructions.md`; reviewed head
`b1579be` (reviewed as the working tree at that content) / base `trunk`
`4110349`; evidence: `npm run lint` clean, `npm test` 1284 pass 2 skip 0
fail, `npm run test:e2e:packaged` 11/11 on macOS against a build made
with the new list; outcome below.
- **Since review:** `4110349` → `b1579be` → `c7f430d` → `010ec0e` →
`23b41f9` (trunk merged in from GitHub, #449 and #452, no overlap with
this diff) → `7a40ce8`. `7a40ce8` trims the native module's Windows
build leftovers found while inspecting the signed artifact, and turns
the native-module check from resolve into load; lint, the unit suite
(1285 pass) and the packaged suite (15/15 on macOS) re-run on it. The
self-review ran against the working tree that became `b1579be`, plus the
two fixes listed below. `c7f430d` adds the npm CLI assertion, written
while checking the runtime paths the allow-list could drop. `010ec0e`
answers CodeRabbit's one finding. Each step was re-run through lint, the
unit suite and the packaged suite (13/13 on macOS at `010ec0e`).
- **Review:** completed — delta pass on `c7f430d`, `010ec0e` and
`7a40ce8` (the commits no bot reviewed), judgement in a fresh subagent
context against `.github/instructions/code-review.instructions.md`, at
head `64d9bd0` (trunk merged in, no overlap). 0 [fix here] · 1
[follow-up] · 1 style. Follow-up: `tests/e2e/packaged/smoke.spec.js:87`
runs the host `git` for `ls-files` instead of the bundled binary through
`tests/unit/helpers/git.cjs`; a machine without Git on PATH fails the
packaged suite for a reason unrelated to packaging. Style: the
bundled-Git test's doc block sits above the native-module block, apart
from its test. Both left for a follow-up. CI on `64d9bd0`: all seven
checks green on both platforms.
- **Review:** completed — CodeRabbit, on `c7f430d` only. Its incremental
review of `010ec0e` did not run: "Review rate limited", and it replies
that it does not re-review already-reviewed commits. So the last commit
carries the self-review and the CI evidence, not a bot pass. Full detail
on `c7f430d`: Its first two runs did not happen: the check read `pass`
with the message "Review rate limited". The third ran. One Major
finding, fixed in `010ec0e` and answered inline: `src/**/*` is recursive
and nothing asserted the nested contents. It proposed enumerating the
runtime files by hand; the fix derives the expected set from Git
instead, which closes the same gap without a list that has to be
maintained. Verified by dropping a file into `src/`, packaging, and
watching the test name it.

Fixed here:

1. *The drift guard was one-directional*
(`tests/e2e/packaged/smoke.spec.js`). Removing a pattern from
`build.files` failed nothing. Added a test that derives the allowed set
from `package.json` and asserts it equals the literal list in the spec.
Verified by deleting `local-playground-web/**/*` from package.json: the
test fails naming that entry.
2. *The one negative pattern was unverified* (same file). Both payload
assertions read only tree roots, so `!src/renderer/index.jsx` could stop
matching silently. The asar test now also lists `src/renderer` and
asserts `index.jsx` is absent while `index.html`, `index.js` and
`index.css` are present — which doubles as a check that the renderer
bundle was built before packaging.

Deferred, with reason:

3. *The Linux window icon* (`src/main.js:490`) resolves to
`<app.asar>/build/icon.png`, which has never shipped: electron-builder
excludes `directories.buildResources` unconditionally, so the old
exclusion list did not carry it either. Pre-existing and Linux-only, and
Linux formats are explicitly outside the v1.1.0 validation scope (#388).
Recorded in Risks rather than filed as an issue.

Style notes from the same pass, all applied: the stale `vendor` sentence
in `.gitignore:33` now points at the allow-list; the two failure
messages named the whole listing instead of the offenders; the
local-equivalence claim in `TESTING.md` was unconditional and is now
hedged (`.codesigning` exists only on a signing machine, so the full
proof is the CI run). The `dist/intermediate` seed in the workflow was
noted as redundant — electron-builder already excludes its own output
directory — and kept deliberately: it is the seed that documents *why* a
top-level `dist/` is safe, and it guards a future re-introduction of a
permissive pattern.

</details>

<details>
<summary>Implementation notes</summary>

- electron-builder 26.15.3,
`node_modules/app-builder-lib/out/fileMatcher.js`: `getMainFileMatchers`
adds `package.json` when the list is positive, prepends
`!**/node_modules/**`, `!<buildResources>{,/**/*}` and
`!<output>{,/**/*}`, and appends the default name and extension
excludes. `getNodeModuleFileMatcher` takes only the `!` patterns from
`files`, which is why the dugite trims keep working.
- Baseline asar root on trunk `4110349` (macOS, local, seeded):
`node_modules`, `tests`, `src`, `docs`, `scripts`, `.buildkite`,
`.claude`, `.agents`, `test-results`, `fastlane`, `.bundle`,
`TESTING.md`, `STATS.md`, `README.md`, `playwright.config.js`,
`package.json`, `LICENSE`, `Gemfile.lock`, `Gemfile`,
`eslint.config.mjs`, `CONTRIBUTING.md`, `CLAUDE.md`, `CLAUDE.local.md`,
`AGENTS.override.md`, `AGENTS.md`, `.ruby-version`, `.nvmrc`,
`.coderabbit.yaml`. Absent: top-level `dist`, `.codesigning`, `build`,
`vendor`.
- With the allow-list: `node_modules` (28538 entries, unchanged), `src`
(81), `package.json`. `app.asar.unpacked` holds only `node_modules`
(`@php-wasm`, `dugite`, `fs-ext-extra-prebuilt`).
- Local-versus-CI tree comparison: recorded in a comment once the CI
artifact for this head exists.

</details>

<details>
<summary>Screenshots or recording</summary>

Nothing on screen changed: this PR touches packaging and a test.

</details>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01XJUaihwanFHxEps959WLhs

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants