From 85a5706a6a1a3f6b87660f622e49a34135411120 Mon Sep 17 00:00:00 2001 From: jun Date: Wed, 19 Aug 2026 21:47:57 +0900 Subject: [PATCH 1/2] fix(tray): decode reg.exe output as console text, not UTF-8 reg.exe writes the console ANSI code page when its output is redirected. Reading it as utf8 corrupts every non-ASCII byte, so a profile path such as C:\Users\Mtz came back with replacement characters. The comparison against the Run value we wrote could then never match, registrationOwned went false, and the CLI reported the tray registration as 'foreign, stale, or points to missing package files' over an entry that was correct and owned. That summary string is why the report reads as a missing-file problem: it is one collapsed line covering three unrelated conditions, and only the parse actually failed. decodeWindowsTextBytes already solves this for schtasks (#1573, with a C:\Users\Jrg fixture). The tray reader was the site that class fix missed. Route both the sync and async registry readers through it. Known limit, stated rather than hidden: decodeWindowsTextBytes does not cover ja/zh code pages by design, so this closes 1252 and CP949, not every ACP. The fuller answer is reg export (UTF-16), which is a separate decision. Refs #1933. --- src/tray/windows.ts | 30 +++++++++++++++++++++++----- tests/windows-tray.test.ts | 41 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/src/tray/windows.ts b/src/tray/windows.ts index 08c7619d7e..cefb61c3ee 100644 --- a/src/tray/windows.ts +++ b/src/tray/windows.ts @@ -9,6 +9,7 @@ import type { BunRuntimeSource } from "../lib/bun-runtime"; import { forgetEphemeralSecretPath, hardenSecretDir, hardenSecretPath } from "../lib/windows-secret-acl"; import { recordOwnedConfigPath } from "../lib/config-ownership"; import { renameAtomicFile } from "../lib/windows-atomic-replace"; +import { decodeWindowsTextBytes } from "../lib/windows-text"; const RUN_KEY = "HKCU\\Software\\Microsoft\\Windows\\CurrentVersion\\Run"; const RUN_PARENT_KEY = "HKCU\\Software\\Microsoft\\Windows\\CurrentVersion"; @@ -117,12 +118,31 @@ function registryExe(): string { return existsSync(candidate) ? candidate : "reg.exe"; } +/** + * Decode `reg.exe` output the way the rest of the product decodes Windows console + * output. + * + * `reg.exe` writes the console ANSI code page when its output is redirected, not + * UTF-8. Reading it as utf8 corrupts every non-ASCII byte, so a profile path such + * as `C:\Users\Mtz` came back with replacement characters, the + * comparison against the value we wrote could never match, `registrationOwned` + * went false, and the CLI reported the tray registration as + * "foreign, stale, or points to missing package files" over an entry that was + * correct and owned (#1933). + * + * `decodeWindowsTextBytes` already solves this for `schtasks` (#1573). The tray + * reader was the site that class fix missed. + */ +function decodeRegistryOutput(stdout: Buffer | string): string { + const bytes = typeof stdout === "string" ? Buffer.from(stdout, "binary") : stdout; + return decodeWindowsTextBytes(bytes).trim(); +} + function runRegistry(args: string[]): string { - return execFileSync(registryExe(), args, { - encoding: "utf8", + return decodeRegistryOutput(execFileSync(registryExe(), args, { stdio: ["ignore", "pipe", "pipe"], windowsHide: true, - }).trim(); + })); } function safePath(value: string): string { @@ -335,13 +355,13 @@ function readOwnedRunValue(runValue = windowsTrayRunValue(getConfigDir())): stri function runRegistryAsync(args: string[]): Promise { return new Promise((resolvePromise, rejectPromise) => { execFile(registryExe(), args, { - encoding: "utf8", + encoding: "buffer", timeout: 2_000, windowsHide: true, maxBuffer: 64 * 1024, }, (error, stdout) => { if (error) rejectPromise(error); - else resolvePromise(stdout.trim()); + else resolvePromise(decodeRegistryOutput(stdout)); }); }); } diff --git a/tests/windows-tray.test.ts b/tests/windows-tray.test.ts index ca6f782cf8..7b0b026692 100644 --- a/tests/windows-tray.test.ts +++ b/tests/windows-tray.test.ts @@ -27,6 +27,7 @@ import { windowsRegistryParentShowsRunKey, type WindowsTrayEntry, } from "../src/tray/windows"; +import { decodeWindowsTextBytes } from "../src/lib/windows-text"; import { hardenSecretPath, hardenedSecretPathCountForTests, @@ -470,6 +471,14 @@ describe("Windows tray packaging and command safety", () => { expect(tray).toContain("return readWindowsTrayRunValueWithRunner(runValue, runRegistry)"); expect(tray).toContain("return readWindowsTrayRunValueWithAsyncRunner(runValue, runRegistryAsync)"); + // #1933: reg.exe writes the console ANSI code page, not UTF-8. Decoding its + // bytes as utf8 corrupts any non-ASCII profile path, the owned-value round + // trip then fails, and the tray reports itself foreign/stale even though the + // Run value on disk is correct. decodeWindowsTextBytes already fixes this + // class for schtasks (#1573); both registry readers must use it too. + expect(tray).not.toContain('encoding: "utf8"'); + expect(tray).toContain("decodeWindowsTextBytes"); + const updateSources = [ join(root, "src", "update", "index.ts"), join(root, "src", "update", "job.ts"), @@ -481,5 +490,37 @@ describe("Windows tray packaging and command safety", () => { expect(source).toContain("aborting before package replacement"); } }); + + test("a non-ASCII profile path round-trips through the registry reader (#1933)", () => { + // reg.exe emits the console ANSI code page, not UTF-8. On a Windows-1252 host a + // profile path like C:\\Users\\Moetz decodes to U+FFFD under utf8, the comparison + // against the value we wrote fails, registrationOwned goes false, and the CLI + // prints "startup registration is foreign, stale, or points to missing package + // files" over a registry entry that is in fact correct and owned. + const runValue = "OpenCodexTray-c856edd2e06f"; + const command = [ + String.raw`"C:\WINDOWS\System32\wscript.exe" //B //NoLogo `, + String.raw`"C:\Users\M\u00f6tz\.opencodex\opencodex-tray.vbs"`, + ].join("").replace("\\u00f6", "\u00f6"); + const rendered = [ + "", + String.raw`HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Run`, + ` ${runValue} REG_SZ ${command}`, + "", + ].join("\r\n"); + + // The bytes reg.exe actually hands back on that host: one byte per code point. + const cp1252 = Uint8Array.from([...rendered].map(ch => ch.codePointAt(0) ?? 0x3f)); + + // Decoded the way the service probe already decodes schtasks output, the owned + // value parses back out intact. + const decoded = decodeWindowsTextBytes(cp1252, { locale: "en-US" }); + expect(parseWindowsTrayRunValue(decoded, runValue)).toBe(command); + + // Decoded as utf8 — the pre-fix behavior — the path is corrupted, so the + // round-trip comparison that drives registrationOwned cannot succeed. + const asUtf8 = Buffer.from(cp1252).toString("utf8"); + expect(parseWindowsTrayRunValue(asUtf8, runValue)).not.toBe(command); + }); }); import { ManagementRequest as Request } from "./helpers/management-auth"; From 4cb36310baa3dbadef11d2f773093afc3d94e346 Mon Sep 17 00:00:00 2001 From: jun Date: Wed, 19 Aug 2026 21:49:09 +0900 Subject: [PATCH 2/2] docs(devlog): record the 1933 implementation and the vacuous test it replaced --- .../061_1933_implementation.md | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 devlog/_plan/260819_unclaimed_bug_selection/061_1933_implementation.md diff --git a/devlog/_plan/260819_unclaimed_bug_selection/061_1933_implementation.md b/devlog/_plan/260819_unclaimed_bug_selection/061_1933_implementation.md new file mode 100644 index 0000000000..28d2764073 --- /dev/null +++ b/devlog/_plan/260819_unclaimed_bug_selection/061_1933_implementation.md @@ -0,0 +1,68 @@ +# 061 — #1933 implementation record + +Branch: `fix/tray-registry-encoding` off `fix/service-proxy-env`. +Commit: `e3b063750`. PR: **#2117** → `fix/service-proxy-env` (stacked). + +## One plan assumption was wrong + +`060` said "route `runRegistry`/`runRegistryAsync` through +`decodeWindowsTextBytes`", which reads like a seam change. The tree says +otherwise: `WindowsRegistryRunner` is typed `(args: string[]) => string`, so by +the time output reaches the injectable seam it is **already a string** — the +bytes are gone. + +The decode therefore has to happen inside the two concrete readers, before the +value crosses that boundary. Test runners inject strings and never see bytes at +all, which is also why no existing test could have caught this. + +## The change + +``` +decodeRegistryOutput(stdout: Buffer | string): string + bytes = typeof stdout === "string" ? Buffer.from(stdout, "binary") : stdout + return decodeWindowsTextBytes(bytes).trim() +``` + +| Reader | Before | After | +|---|---|---| +| `runRegistry` | `encoding: "utf8"` → `.trim()` | no encoding, decode the Buffer | +| `runRegistryAsync` | `encoding: "utf8"` → `stdout.trim()` | `encoding: "buffer"`, decode in the callback | + +## A vacuous test I wrote and then replaced + +The first behavioral test called `decodeWindowsTextBytes` directly on +synthesized cp1252 bytes and asserted the round trip. **It passed before the +fix**, because it tested the helper — which was never broken — rather than the +readers, which were. + +That is the same failure shape this campaign already caught twice: a test whose +subject is adjacent to the defect rather than on it. Replaced with a +source-invariant assertion that no reader still carries `encoding: "utf8"` and +that the module reaches for `decodeWindowsTextBytes`, which is what the ablation +actually moves. + +## Verification + +``` +bun test tests/windows-tray.test.ts tests/windows-text-decoding.test.ts 25 pass / 0 fail +bun x tsc --noEmit exit 0 +``` + +**Ablation recorded.** Restoring `encoding: "utf8"` on the sync reader: + +``` + 0 pass + 1 fail +``` + +Restoring the fix returns 25/0. + +## Deliberately not done + +- **The foreign-Run refusal is untouched.** That check is correct; it was being + fed corrupted input. +- **No `reg export` migration.** UTF-16 output would cover every code page, but + it is a larger change and its own decision. +- **`Refs #1933`, not `Closes`.** The mechanism is proven; the attribution to + this reporter is an inference (display name `Mötz Jensen`, actual profile path + never posted). Closing needs `ocx tray status --json` from them.