Files
safetynet/docs/electron-best-practices.md
T

116 lines
7.3 KiB
Markdown
Raw Normal View History

# Electron quality and performance review
The current Windows DLP MVP keeps security enforcement as the priority. This
review uses the official Electron documentation as the implementation authority:
[security](https://www.electronjs.org/docs/latest/tutorial/security),
[performance](https://www.electronjs.org/docs/latest/tutorial/performance),
[context isolation](https://www.electronjs.org/docs/latest/tutorial/context-isolation),
[preloads](https://www.electronjs.org/docs/latest/tutorial/tutorial-preload),
[IPC](https://www.electronjs.org/docs/latest/tutorial/ipc),
[coding style](https://www.electronjs.org/docs/latest/development/coding-style)
and [fuses](https://www.electronjs.org/docs/latest/tutorial/fuses).
## Decisions and implementation
- Keep sandboxing, context isolation, no page Node integration, constrained
preload APIs, sender/frame validation and input validation. These need runtime
and negative tests; configuration alone does not prove a secure browser.
- Add pinned ESLint tooling and an explicit Node/browser environment contract.
Do not hide findings through blanket rule disabling. Dependency advisory
results and lint completion are tracked separately from syntax checks.
- Profile before optimizing. `scripts/profile-browser-modules.mjs` works with
Node CPU/heap profiling flags; output belongs in ignored `artifacts`.
- Defer OpenTelemetry SDK/exporter loading until diagnostics are authorized
and enabled. Keep the lightweight diagnostics API available immediately.
This shifts initialization cost to first enablement; it does not remove that
cost or claim a measured improvement to full browser startup. Seventeen focused
diagnostics regression tests pass after this change.
- Load text-normalization code on its first inspection rather than immediately
upon requiring its wrapper; subsequent calls reuse the module.
- Keep parsing, OCR and model inference in the isolated worker. Do not move
expensive inspection into the renderer or postpone mandatory enforcement.
- Review main-process synchronous filesystem/cryptographic operations using
traces before changing behavior. Build CLI `spawnSync` is separate from the
interactive browser event loop.
## Bundling and architecture
[Electron Forge](https://www.electronforge.io/) and the
[official bundling recommendation](https://www.electronjs.org/docs/latest/tutorial/performance)
are useful options. The existing electron-builder pipeline already verifies
Windows runtime contents. Migrating scaffolding is not itself a DLP improvement.
Any bundling/code-splitting change must retain sandbox-compatible preloads,
dynamic imports, packaged resources, CSP and exact package verification, and
must show measured startup or resource improvements before adoption.
The [Palette performance article](https://palette.dev/blog/improving-performance-of-electron-apps)
was inspected for additional profiling ideas. Type-safe IPC examples are useful
for maintainability; TypeScript types cannot replace runtime validation of
untrusted messages. No new IPC framework is needed merely to implement that rule.
## Source freshness
The dependency audit reports zero production advisories and eight moderate
development/build-toolchain entries, with no high or critical entries at this
review point. The underlying `sprintf-js` precision-format denial-of-service
advisory has no patched release reported; suggested electron-builder downgrade
is not accepted as a safe remediation. These findings remain explicit tooling
risk, not evidence that the deployed browser is vulnerability-free.
The packaged executable's existing fuse wire was inspected separately from
JavaScript startup checks. Run-as-Node, Node environment/CLI inspection options
and ASAR enforcement need package-level hardening and rebuilt-artifact proof.
All six hardened fuse values and the matching embedded ASAR header hash were
verified in a rebuilt executable. Actual RUN_AS_NODE and NODE_OPTIONS negative
launches passed. The CLI-inspector fixture found a startup error dialog instead
of process termination; the source now exits with a sanitized rejection, but
the corrected rebuilt package subsequently passed actual negative launches,
including eleven certificate/feature override cases and copied-ASAR tampering.
The manipulated archive was rejected before application entry; original archive
and frozen packaged sources remained unchanged. Evidence:
`artifacts/package-fuses-proof.json`. Actual signing status remains `NotSigned`.
The supplied Reasonably Secure Electron URL contained a typographic dash.
Its correct repository is
[moloch--/reasonably-secure-electron](https://github.com/moloch--/reasonably-secure-electron),
archived in June 2024; treat it as historical architecture context rather than
current Electron defaults. The supplied dev.to performance page could not be
retrieved and was not used as implementation evidence. Other supplied articles
were inspected, but version-specific changes must be checked against official
documentation and the actual installed runtime.
This review is in progress. Green syntax/tests and one profiling run do not prove
that the whole application is clean, fully optimized or independently audited.
## Latest reproducible checks
- Full source suite: 749 tests passed, no failures or skips; see the ignored
`artifacts/mvp-final-startup-suite.log`. `npm run lint` passed again with
zero warnings on 2026-10-13.
- A fresh `npm audit --omit=dev --json` reported zero advisories. Development
toolchain findings remain tracked separately in `dependency-audit.md`.
- In the paired single module-load profiles, diagnostics require time changed
from 260.119 ms to 67.001 ms after deferred SDK loading. Recorded heap deltas
changed from 6,674,088 to 2,460,832 bytes. Both used CPU/heap profiling; these
measurements do not establish full application startup or percentile gains.
- Native Windows DLP fixtures delivered public data and blocked confidential
literal and semantic payloads. Restart reused the protected device profile.
These are separate from final packaged-artifact acceptance.
- The protected local OpenTelemetry receiver reported zero rejected writes.
Trace `33b46998b44c3179cccd2826d5790c00` correlates browser.request,
browser.inspect.local, inspector.inspect, semantic load/inference and durable
audit stages. The public-file request took 8966 ms in this synthetic run;
a contained local text scan took 1611 ms and model load 662 ms. This identifies
repeated model initialization as a measurement target, not permission to
weaken process isolation or mandatory scanning.
- A native WebRTC initial-about:blank renderer crash remains reproducible under
Chromium's connection allowlist. The upstream no-op binder fix is recorded
in [the upstream regression tests](https://github.com/web-platform-tests/wpt/pull/63420).
The separately tested Electron nightly also lacks it. The Windows MVP is not
declared stable until an actual fixed runtime passes this negative fixture.
- The full enrolled browser WebTransport receiver fixture now passes through
reviewed DIRECT and fetched HTTPS PAC policies: zero managed packets/bytes,
paired against 132 baseline bytes, with ordinary HTTPS submissions working.
It records source hashes and genuine operational traces. There is still no
per-attempt WebTransport security event because this path produces no
Electron webRequest callback; transport denial and audit coverage are separate.