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

182 lines
12 KiB
Markdown

# 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
The latest final-source Windows acceptance supersedes the historical failed
runs below:784 tests, lint, packaged-artifact verification, actual clipboard,
upload, process restart, local Gemma and audited Window/Worker transport tests
pass. All native browser lifetimes have zero export failures. Bounded exporter
FIFO, an SDK completion barrier, single-flight flush, verified policy continuity,
collector admission and idempotent private ACL startup are implemented.
See [current evidence and limits](diagnostics-backpressure.md). The separate
WebRTC engine crash remains reproducible in the official44.7.1 candidate too.
A subsequent full run after the Established-alarm regression added a test
passed 764 of765 and exposed a real gateway cancellation race: the policy
monitor aborted the provider but discarded its fixed409 error, producing500.
The gateway now preserves the first fixed cancellation reason (policy409,
caller403, deadline504). Fourteen focused tests and scoped lint passed. Actual
collector trace `3b6b5e374da506f7ae5ffbb118933bf6` shows the provider aborted at
101.6ms under the gateway and library parents. The earlier failed full run is
retained in `artifacts/transport-owner-containment-final-suite.log`.
The subsequent native clipboard/upload scenario passed all17 enforcement
checks but failed diagnostics acceptance because lifetime exports recorded two
errors. Collector rejected writes remained0; their exact causes cannot be
reconstructed from the original counters. Failure instrumentation and a fresh
acceptance run must resolve this separately from DLP transmission proof.
The browser now retains at most32 cloned export-failure facts and lifetime
totals by fixed reason. Facts include traces/logs, generation, duration, fixed
HTTP status, retirement overlap and explicitly signalled cancellation. Raw
error text, response bodies, credentials and collector URLs are excluded.
Actual held HTTP exports prove that retirement cancellation remains counted
as failure; the instrumentation does not silently reset or retry operations.
Seventeen focused diagnostics tests passed, including timeout, refusal,503,
bounded history and concurrent flush attribution.
A fresh public-only native diagnostic run completed with four independently
reviewed policy publications and four public HTTPS submissions. Every snapshot
had zero current/lifetime failures and an empty failure history; actual exported
trace `b5eefaf6bb8407f6917bd5cada6e8587` and collector rejectedWrites0 were
verified. Reproduction: `node scripts/native-diagnostics-public.mjs` with the
protected local collector already running. This does not retrospectively prove
the cause of the earlier two failures or reset their historical evidence.
A further quiet public-only native measurement delivered80 benign HTTPS
responses across four reviewed publications: median59.53ms, p95 83.19ms,
max88.17ms; observed main event-loop maximum drift328.29ms over637 samples.
Current/lifetime failures remained0, collector rejectedWrites0. Trace
`d4689e3d28ebe902b752500c0f79e30b` and
`artifacts/public-backpressure-native-proof.json` are local evidence. This does
not resolve or causally explain the semantic run's three timeouts and one
concurrency failure. A bounded export queue, single-flight flush and collector
commit-latency measurements are the next narrowly scoped diagnostics work;
mandatory inspection and audit durability must remain intact.
- Full source suite: 784 tests passed with `--test-concurrency=2`, no failures
or skips; see `artifacts/diagnostics-continuity-final-suite.log`. An earlier
higher-concurrency run failed one evidence-key ACL check; its cause remains
unconfirmed and its log is retained. All 18 evidence tests passed separately.
No ACL protection or timeout was weakened. `npm run lint` passed with zero
warnings after the final source changes.
- 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 packaged-artifact acceptance. The final literal and
actual-restart fixtures pass17+5 checks with zero lifetime exports failed.
The latest semantic receiver/model and diagnostics checks pass with zero
lifetime failures. Earlier three measured2s timeouts and one concurrency
rejection remain retained as real failures, with no retrospective cause claim.
- 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. A fixed private
native channel now shares the existing clipboard debugger owner and records
six Created and six Closed security events across the tested window and
dedicated-worker attempts, with zero Established events. All twelve events
correlate with exported audit traces. No raw URL, payload or CDP identifier
enters these events. See [native transport observability](native-transport-observability.md)
for coverage limits, failure behavior and ECS/CEF mapping. The private relay
remains the transmission boundary; lifecycle observations alone do not prove
content inspection or universal realm coverage.