Skip to content

fix(desktop): reject empty packaged app payloads - #71523

Open
yinkev wants to merge 3 commits into
NousResearch:mainfrom
yinkev:fix/windows-desktop-exe
Open

yinkev wants to merge 3 commits into
NousResearch:mainfrom
yinkev:fix/windows-desktop-exe

Conversation

@yinkev

@yinkev yinkev commented Jul 25, 2026

Copy link
Copy Markdown

Summary

  • extend the Windows post-build integrity gate beyond the PE executable to the adjacent Electron app payload;
  • reject a tiny/empty resources/app.asar, missing archive entry metadata, or missing unpacked runtime entry files before the update is stamped or launched;
  • apply the same package-level validation to rollback backups so Hermes never restores another non-bootable tree;
  • preserve the deliberate signAndEditExecutable: false configuration and its direct rcedit branding path.

Fixes #70825.

Root cause

A structurally valid Hermes.exe can still launch Electron's help screen when its packaged application is absent. The existing #71119 gate validates the PE header, architecture, and truncation, but it cannot detect the reporter's second failure shape: a 176-byte app.asar with no usable package entry point.

signAndEditExecutable is not the app archive builder and remains intentionally disabled to avoid electron-builder's winCodeSign extraction path. The missing invariant was package validation after the build.

Validation performed

The gate now requires:

  • a nontrivial resources/app.asar;
  • package.json and electron-main.mjs entry metadata in the ASAR header;
  • non-empty app.asar.unpacked/dist/electron-main.mjs and dist/index.html, matching the committed asarUnpack: ["dist/**"] contract.

Verification

  • regression was red on current main: a valid PE with a five-byte app archive was accepted;
  • complete desktop integrity suite: 29 passed;
  • Ruff, py_compile, Windows-footgun scan, and git diff --check: clean.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) platform/windows Native Windows-specific behavior or breakage area/install-update Installer, updater, packaging, wheels, doctor P2 Medium — degraded but workaround exists sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 25, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for extending the existing Windows post-build gate rather than changing the deliberate signAndEditExecutable: false configuration. The premise remains present on current main: hermes_cli/main.py:6171 accepts any PE that passes _desktop_exe_integrity_error(), whose scope is limited to PE structure and architecture at hermes_cli/main.py:6099.

Suggested changes

  • Please add branch-level regression coverage for the new payload checks. The added test covers the tiny-archive case, but the new implementation also rejects missing ASAR metadata and missing/empty app.asar.unpacked/dist runtime files. The repository packaging contract supports these checks: apps/desktop/package.json:9 sets dist/electron-main.mjs as the main entry and apps/desktop/package.json:211 unpacks dist/**.

The implementation otherwise fits the existing rollback gate: it validates both the newly built package and the backup before restoration. This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Three PRs address or reference this issue complex across two related Windows failure modes. #69234 adds pack-time PE architecture validation for the corrupted or wrong-architecture executable reported in #69179; #70976 removes the deliberate Windows executable-editing configuration; #71523 extends the existing gate to validate the Electron application payload implicated by #70825's help-text symptom.

Related pull requests

Duplicates

No substantive duplicates: #69234 covers PE architecture, #70976 proposes a build-configuration change, and #71523 covers the packaged Electron payload and rollback gate.

Suggested consolidation

Keep #71523 open with a salvage path: preserve its payload and rollback checks, then add the branch-level regression coverage requested by the maintainer-bot keep_open review. Keep #69234 and #70976 closed rather than merging them: #69234 is superseded by the recovery implementation recorded in the #69179 discussion via #71119 and #71218, while #70976 conflicts with the documented deliberate local-rebuild configuration; no PR is merge-lane eligible here.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I70825(["issue #70825 (open)"])
    P71523["PR #71523 (open)"]
    P71523 -->|best fix| I70825
    class I70825 open
    class P71523 open
    class P71523 best
    class P71523 target
    click I70825 "https://github.com/NousResearch/hermes-agent/issues/70825"
    click P71523 "https://github.com/NousResearch/hermes-agent/pull/71523"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 3 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 14 kB of PR diffs, 14 kB of issue/PR text, 9 kB of discussion (6 comments), 4 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Windows] Desktop app broken: electron-builder produces invalid Hermes.exe (shows Electron help text)

4 participants