Skip to content

fix(privacy): redact device serials in Gnirehtet native process logs (#58) - #59

Draft
man612 wants to merge 11 commits into
mainfrom
fix/issue58-gnirehtet-log-privacy-20261025
Draft

man612 wants to merge 11 commits into
mainfrom
fix/issue58-gnirehtet-log-privacy-20261025

Conversation

@man612

@man612 man612 commented Oct 24, 2026

Copy link
Copy Markdown
Owner

Goal

Reduce the exposure of Android ADB device identifiers in Wirebound's Gnirehtet terminal logs. Upstream Rust Gnirehtet may include a complete adb -s DEVICE command in native stderr on subprocess failures, and the previous Wirebound handleOutput() forwarded it directly to the renderer. Source audit only; no real user's serial or logs have been accessed.

Related: issue #58. This is an isolated Draft PR based on main and does not include unrelated fixes.

Changes

  • New gnirehtetLogPrivacy.ts: mask known ID strings and recognized Gnirehtet/ADB argument forms, including Rust command arrays, adb -s, labeled IDs and quoted ADB transport errors.
  • Bound separate stdout/stderr line buffers per child process (8192 chars/line). Reassemble data chunks before redacting; drop oversize lines rather than leaking unsafe partial fragments; flush trailing output on error/close.
  • Apply the redactor at all GnirehtetService-emitted log paths and mask returned per-device ActionResult.error values.
  • Add 6 unrun native Vitest unit cases, 3 unrun fake-child-process scenario cases, and permanent QA/agent handoff. No dependency/binary, ADB global server, reverse mapping, app installer or user profile changes.

Real verification vs pending

  • 16/16 isolated pure-JS helper/buffer assertions passed on the freshly fetched GitHub implementation after fixing the adb -s pattern.
  • 9/9 additional isolated GnirehtetService method-body checks passed using fake native process/stdout/stderr and fake ADB. The initial harness had an invalid ordering (action invoked after fake close), corrected without code changes.
  • These are not native Vitest, TypeScript typecheck, build, Windows Electron, NSIS, real Gnirehtet binary or actual Android tests. All of those remain pending until the Windows QA machine is available.

Critical review risks

Draft PR #55 modifies the same src/main/services/gnirehtetService.ts for engine lifecycle cancellation and stale process generation protection. A later integration must preserve both #55's process identity and #58's per-child buffers/redacted logs; never take one entire file as the conflict winner. Separate ADB warning privacy changes are in Draft PR #57.

This is not universal PII sanitization: arbitrary third-party text, unrelated IPs/paths/hostnames and as-yet-unknown unstructured IDs are out of scope. UX PR #50's Copy Support Report uses a separate structured diagnostic report, not raw terminal log output. Do not claim privacy is comprehensive or merge/release based on synthetic checks.

QA checkpoint and detailed native validation gates.

Keep Draft; no merge/release until exact-head native test, isolated Windows process smoke, and one-phone USB/VPN parity checks.

man612 commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

2026-10-25 merge-readiness audit: exact HEAD CI is RED (four TypeScript errors)

Current Draft PR HEAD 89c3881be1faf6eef0151339fb1b6e6bd2574995 has Windows CI run 40141816698, job 121014962963. It installed deps and lint passed, but Typecheck failed with four TS2339 errors:

  • src/main/services/gnirehtetService.scenario.test.ts:195:18
  • src/main/services/gnirehtetService.scenario.test.ts:197:18
  • src/main/services/gnirehtetService.scenario.test.ts:215:18
  • src/main/services/gnirehtetService.scenario.test.ts:216:18

All errors: Property 'write' does not exist on type 'Readable'. The fake child streams are implemented with PassThrough but their signature is ChildProcessWithoutNullStreams, whose stdout/stderr members are typed Readable. Remedy using appropriate test-only typing/cast to PassThrough, or emit synthetic data events via a correctly typed fake process. Avoid weakening production types. Unit tests, build and package smoke were skipped, not passed. These are genuine compile errors, not Actions quota problems.

Recommendation: keep PR #59 Draft/unmerged until fixed and exact-head Windows CI is green, then review same-file integration conflicts with lifecycle PR #55 in gnirehtetService.ts and its scenario test. No source edits or merge performed in this audit.

man612 commented Oct 28, 2026

Copy link
Copy Markdown
Owner Author

2026-10-28 CI blocker patch and QA boundary (unmerged)

Fixed all four previously logged TS2339: Property 'write' does not exist on type 'Readable' sites in src/main/services/gnirehtetService.scenario.test.ts at updated HEAD ca6c1dcb7af9aca40840215ae4fe81c88bf6900a. The scenario now calls child.stdout/stderr.emit('data', Buffer.from(...)) to simulate native Readable data events directly; this matches the service's data listeners without unsafe production type casts. Only scenario test code changed; no Gnirehtet process, ADB or privacy implementation edits.

No GitHub Actions run/check is currently associated with the updated SHA. The same isolated Windows F: QA checkout was used for an attempted native test setup, but installing its dependencies and then launching focused lint using a junction to pre-existing QA node_modules became very slow and was deliberately terminated. It produced NO completed native lint/TypeScript/Vitest result. No actual Android process, VPN, device ID or Wirebound user profile was touched.

Thus four source-level type errors addressed, not yet proven by typecheck. This PR stays Draft; exact-head CI is required. Critical integration: lifecycle PR #55 edits the same gnirehtetService.ts and scenario file. Preserve both lifecycle process-identity/cancellation safeguards and this PR's per-child stdout/stderr buffering and redaction; do not wholesale overwrite either. Current main progressed through #57/#61/#62 so its AGENTS.md contents must also be retained. No merge/release occurred.

man612 commented Oct 28, 2026

Copy link
Copy Markdown
Owner Author

2026-10-28 integration validation: Draft PR #63, head 62d0a800, combines this branch's native log privacy/redaction/buffering with lifecycle PR #55 on actual latest main. Hosted Windows CI #40624716806 has PASS on lint, typecheck, all 62/62 Vitest tests (including 14 service scenarios and six helper tests), app build and package smoke. This confirms the corrected .emit('data', Buffer.from(...)) fixture typechecks and runs in the integration branch. Separately, temporary QA PR #65 is checking this PR's exact source commit ca6c1dc against its original main baseline, because this old PR's merge ref is now dirty. Do not infer all unknown third-party PII patterns are sanitized or that real hardware has been validated. PR #63/PR #59 remain Draft/unmerged.

man612 commented Oct 28, 2026

Copy link
Copy Markdown
Owner Author

Exact-head Windows CI verifier PASS — 2026-10-28

The corrected source HEAD ca6c1dcb7af9aca40840215ae4fe81c88bf6900a was checked without changing this PR through temporary QA PR #65, using the source branch's original base 188751e5 so its now-dirty current-main merge ref would not block checks. Hosted Windows CI run #40624871456 completed SUCCESS: lint, TypeScript, 55/55 Vitest tests in 9 files, build, and unpacked Windows package smoke. The four original TypeScript errors due to .write() on Readable are now demonstrably fixed.

Separately, the combined lifecycle/privacy Draft PR #63 on current main passed CI #40624716806 with 62/62 tests, typecheck, build and package smoke. QA #65 is only a temporary evidence vehicle and will be closed without merge. Original PR #59 remains Draft/unmerged and conflicted with current main; integrate through reviewed branch, never override PR #55 lifecycle guards. No real Gnirehtet/USB/VPN/PII completeness or Windows install testing is claimed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant