Skip to content

Commit 8acdc08

Browse files
committed
fix(security): drop accidental service.ts tip merge from ACL pivot
Keep the PR scoped to the timeout-memo + ADMIN_AUTH_TOKEN escape; restore service/winsw to the prior tip minus only the unverified-ACL bake.
1 parent d46851d commit 8acdc08

2 files changed

Lines changed: 14 additions & 235 deletions

File tree

src/lib/winsw.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,6 @@ function runWinsw(args: string[]): string {
165165

166166
/** `install /p` prompts for the service-account password on the console — stdin must be inherited. */
167167
function runWinswInteractive(args: string[]): void {
168-
if (!process.stdin.isTTY) {
169-
throw new Error(
170-
"WinSW install requires an interactive console to prompt for the service account password. "
171-
+ "Run `ocx service install --native` from an elevated Command Prompt or PowerShell window, not a hidden or piped session.",
172-
);
173-
}
174168
execFileSync(winswExePath(), args, { stdio: "inherit" });
175169
}
176170

src/service.ts

Lines changed: 14 additions & 229 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,10 @@
66
* restore it via the command.
77
*/
88
import { execFileSync, execSync } from "node:child_process";
9-
import { findLiveProxy } from "./server/proxy-liveness";
109
import { chmodSync, existsSync, mkdirSync, readFileSync, unlinkSync, writeFileSync } from "node:fs";
1110
import { homedir } from "node:os";
1211
import { dirname, join, resolve } from "node:path";
13-
import { expandUserPath, getConfigDir, readPid, removePid, removeRuntimePort, verifyPidIdentity } from "./config";
12+
import { expandUserPath, getConfigDir, readPid, removePid, removeRuntimePort } from "./config";
1413
import { loadConfig } from "./config";
1514
import { restoreNativeCodex } from "./codex/inject";
1615
import { stripGrokConfig } from "./grok/inject";
@@ -428,70 +427,6 @@ export type WindowsSchedulerTaskProbe =
428427
| { status: "absent" }
429428
| { status: "unknown"; detail: string };
430429

431-
export type WindowsSchedulerProxyProbe =
432-
| { status: "running"; port: number }
433-
| { status: "not-running" }
434-
| { status: "unknown" };
435-
436-
/**
437-
* Render Task Scheduler status without exposing localized `schtasks` table output.
438-
* The task probe answers installation state; the identity-checked health probe answers
439-
* runtime state. Keep probe details out of this user-facing line because they can contain
440-
* incorrectly decoded, locale-specific command output.
441-
*/
442-
export function formatWindowsSchedulerServiceStatus(
443-
task: WindowsSchedulerTaskProbe,
444-
proxy: WindowsSchedulerProxyProbe,
445-
): string {
446-
if (task.status === "present") {
447-
if (proxy.status === "running") {
448-
return `✅ service installed (Task Scheduler); OpenCodex proxy running on port ${proxy.port}.`;
449-
}
450-
if (proxy.status === "not-running") {
451-
return "⚠️ service installed (Task Scheduler); OpenCodex proxy not running.";
452-
}
453-
return "⚠️ service installed (Task Scheduler); OpenCodex proxy status unknown.";
454-
}
455-
if (task.status === "absent") {
456-
if (proxy.status === "running") {
457-
return `❌ service not installed (Task Scheduler); OpenCodex proxy is running independently on port ${proxy.port}.`;
458-
}
459-
if (proxy.status === "unknown") {
460-
return "❌ service not installed (Task Scheduler); OpenCodex proxy status unknown.";
461-
}
462-
return "❌ service not installed (Task Scheduler).";
463-
}
464-
if (proxy.status === "running") {
465-
return `⚠️ Task Scheduler registration unknown; OpenCodex proxy running on port ${proxy.port}.`;
466-
}
467-
if (proxy.status === "not-running") {
468-
return "⚠️ service status unknown (Task Scheduler query failed); OpenCodex proxy not running.";
469-
}
470-
return "⚠️ service status unknown (Task Scheduler and proxy checks failed).";
471-
}
472-
473-
export async function inspectWindowsSchedulerServiceStatus(io: {
474-
probeTask?: () => WindowsSchedulerTaskProbe;
475-
findProxy?: () => Promise<{ port: number } | null>;
476-
} = {}): Promise<string> {
477-
let task: WindowsSchedulerTaskProbe;
478-
try {
479-
task = (io.probeTask ?? probeWindowsSchedulerTask)();
480-
} catch (error) {
481-
task = { status: "unknown", detail: schtasksErrorDetail(error) };
482-
}
483-
484-
let proxy: WindowsSchedulerProxyProbe;
485-
try {
486-
const live = await (io.findProxy ?? findLiveProxy)();
487-
proxy = live ? { status: "running", port: live.port } : { status: "not-running" };
488-
} catch {
489-
proxy = { status: "unknown" };
490-
}
491-
492-
return formatWindowsSchedulerServiceStatus(task, proxy);
493-
}
494-
495430
function schtasksErrorDetail(error: unknown): string {
496431
return error instanceof Error ? error.message : String(error);
497432
}
@@ -1428,38 +1363,7 @@ export async function repairService(deps: RepairServiceDeps = {}): Promise<void>
14281363
* scheduler backend first; on failure the machine is left with NO service (explicitly
14291364
* reported) — never a silent fallback to the scheduler.
14301365
*/
1431-
/** Refuse WinSW when the interactive user is a Microsoft account (SCM cannot authenticate it). */
1432-
export function assertWindowsNativeServiceAccountSupported(): void {
1433-
if (process.platform !== "win32") return;
1434-
const source = readWindowsPrincipalSource();
1435-
if (source?.toLowerCase() === "microsoftaccount") {
1436-
throw new Error(
1437-
"The native (WinSW) service backend cannot run under a Microsoft-account Windows login. "
1438-
+ "Keep the Task Scheduler backend (`ocx service install`) or sign in with a local/domain account before `ocx service install --native`.",
1439-
);
1440-
}
1441-
}
1442-
1443-
function readWindowsPrincipalSource(): string | null {
1444-
if (process.platform !== "win32") return null;
1445-
const ps = join(process.env.SystemRoot ?? "C:\\Windows", "System32", "WindowsPowerShell", "v1.0", "powershell.exe");
1446-
if (!existsSync(ps)) return null;
1447-
try {
1448-
const out = execFileSync(ps, [
1449-
"-NoLogo",
1450-
"-NoProfile",
1451-
"-NonInteractive",
1452-
"-Command",
1453-
"(Get-LocalUser -Name $env:USERNAME -ErrorAction SilentlyContinue).PrincipalSource",
1454-
], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], windowsHide: true }).trim();
1455-
return out || null;
1456-
} catch {
1457-
return null;
1458-
}
1459-
}
1460-
14611366
async function installWindowsNative(): Promise<void> {
1462-
assertWindowsNativeServiceAccountSupported();
14631367
recordOwnedConfigPath(getConfigDir(), serviceStatePath());
14641368
if (!existsSync(getConfigDir())) mkdirSync(getConfigDir(), { recursive: true });
14651369
writeServiceApiTokenFile();
@@ -1494,49 +1398,11 @@ async function installWindowsNative(): Promise<void> {
14941398
writeServiceInstallState("native");
14951399
}
14961400
function startWindows(): void { schtasks(["/run", "/tn", TASK]); }
1497-
1498-
export function isWindowsSchedulerEndBenign(error: unknown): boolean {
1499-
const detail = schtasksErrorDetail(error).toLowerCase();
1500-
return detail.includes("no running instance")
1501-
|| detail.includes("not currently running")
1502-
|| detail.includes("0x41330");
1503-
}
1504-
1505-
/**
1506-
* End the scheduler task. "Already stopped" is success; other `/end` failures are
1507-
* swallowed so callers can still run tracked-proxy + live-proxy cleanup.
1508-
*
1509-
* Do not key a restart-window wait on `/end` failure: the #764 case is an `/end`
1510-
* that *succeeds* while the wrapper survives and respawns. That verification lives
1511-
* on the stop-verification path (poll across the restart window), not here.
1512-
*/
1513-
export function stopWindows(): void {
1514-
try {
1515-
schtasks(["/end", "/tn", TASK]);
1516-
} catch (error) {
1517-
if (isWindowsSchedulerEndBenign(error)) return;
1518-
}
1519-
}
1401+
function stopWindows(): void { try { schtasks(["/end", "/tn", TASK]); } catch { /* not running */ } }
15201402
function statusWindows(): string { try { return schtasks(["/query", "/tn", TASK]); } catch { return ""; } }
15211403
function statusWindowsXml(): string { try { return schtasks(["/query", "/tn", TASK, "/xml"]); } catch { return ""; } }
15221404
function uninstallWindows(): void {
1523-
const probe = probeWindowsSchedulerTask(TASK);
1524-
if (probe.status === "present") {
1525-
try {
1526-
schtasks(["/delete", "/tn", TASK, "/f"]);
1527-
} catch (error) {
1528-
throw new Error(`Failed to delete Task Scheduler task ${TASK}: ${error instanceof Error ? error.message : String(error)}`);
1529-
}
1530-
const afterDelete = probeWindowsSchedulerTask(TASK);
1531-
if (afterDelete.status === "present") {
1532-
throw new Error(`Task Scheduler task ${TASK} is still present after delete — refusing to remove service assets. Retry from an elevated shell.`);
1533-
}
1534-
if (afterDelete.status === "unknown") {
1535-
throw new Error(`Task Scheduler task ${TASK} presence could not be verified after delete — refusing to remove service assets.`);
1536-
}
1537-
} else if (probe.status === "unknown") {
1538-
throw new Error(`Task Scheduler task ${TASK} presence could not be verified — refusing to remove service assets.`);
1539-
}
1405+
try { schtasks(["/delete", "/tn", TASK, "/f"]); } catch { /* absent */ }
15401406
if (existsSync(windowsServiceScriptPath())) unlinkSync(windowsServiceScriptPath());
15411407
if (existsSync(windowsLauncherVbsPath())) unlinkSync(windowsLauncherVbsPath());
15421408
if (existsSync(windowsTaskXmlPath())) unlinkSync(windowsTaskXmlPath());
@@ -1695,77 +1561,18 @@ function platformOps(backend: ServiceBackend = "scheduler"): ServiceOps | null {
16951561

16961562
type TrackedProxyCleanupResult = "none" | "stale" | "stopped";
16971563

1698-
function verifiedKillTarget(pid: number | null | undefined): number | null {
1699-
if (typeof pid !== "number" || !Number.isSafeInteger(pid) || pid <= 0) return null;
1700-
const verified = verifyPidIdentity(pid);
1701-
return verified === pid ? verified : null;
1702-
}
1703-
1704-
/**
1705-
* Whether a proxy is still answering after the service manager claimed to stop it.
1706-
*
1707-
* `ops.stop()` reports the outcome of the STOP COMMAND, not of the process. A Windows scheduler
1708-
* task whose wrapper survives `schtasks /end` respawns its child a few seconds later, so a stop
1709-
* that returned success can still leave a live proxy — and `ocx service stop` then restored
1710-
* native Codex on top of a running one (#764). The tracked-pid cleanup does not catch it either:
1711-
* the respawned child writes a different pid, or none this process knows about.
1712-
*
1713-
* Probed rather than assumed, and bounded. The respawn risk is specific to a supervisor that can
1714-
* restart its child — the Windows scheduler wrapper — so only that case pays the restart window.
1715-
* Everywhere else a single probe answers the question, because nothing is going to bring the
1716-
* proxy back after `launchctl unload` or `systemctl stop`. Making every platform wait 7s on a
1717-
* stop that already succeeded would trade one bug for a worse everyday one.
1718-
*/
1719-
export async function proxyStillLiveAfterStop(deps: {
1720-
findProxy?: () => Promise<{ port: number } | null>;
1721-
sleep?: (ms: number) => Promise<void>;
1722-
now?: () => number;
1723-
/** Whether the stopped supervisor can respawn its child; only then is polling worth the wait. */
1724-
canRespawn?: boolean;
1725-
} = {}): Promise<{ port: number } | null> {
1726-
const findProxy = deps.findProxy ?? findLiveProxy;
1727-
const sleep = deps.sleep ?? ((ms: number) => new Promise<void>(r => setTimeout(r, ms)));
1728-
const now = deps.now ?? Date.now;
1729-
const canRespawn = deps.canRespawn ?? process.platform === "win32";
1730-
const deadline = now() + (canRespawn ? 7000 : 0);
1731-
for (;;) {
1732-
try {
1733-
const live = await findProxy();
1734-
if (live) return live;
1735-
} catch {
1736-
// A probe failure is not proof the proxy is gone; keep polling until the deadline.
1737-
}
1738-
if (now() >= deadline) return null;
1739-
await sleep(1000);
1740-
}
1741-
}
1742-
17431564
async function stopTrackedProxyIfRunning(): Promise<TrackedProxyCleanupResult> {
1744-
let stopped = false;
17451565
const pid = readPid();
1746-
const trackedKillPid = verifiedKillTarget(pid);
1747-
if (trackedKillPid !== null && isProcessAlive(trackedKillPid)) {
1748-
await stopProxy(trackedKillPid);
1749-
removePid(trackedKillPid);
1750-
removeRuntimePort(trackedKillPid);
1751-
stopped = true;
1752-
} else if (pid) {
1566+
if (!pid) return "none";
1567+
if (!isProcessAlive(pid)) {
17531568
removePid(pid);
17541569
removeRuntimePort(pid);
1570+
return "stale";
17551571
}
1756-
// Orphan recovery: the pid file can be missing/stale while the service wrapper keeps
1757-
// a live proxy running — mirror `ocx stop`'s identity-checked findLiveProxy fallback.
1758-
const live = await findLiveProxy({ timeoutMs: 1500 });
1759-
const liveKillPid = verifiedKillTarget(live?.pid);
1760-
if (liveKillPid !== null) {
1761-
await stopProxy(liveKillPid);
1762-
removePid(liveKillPid);
1763-
removeRuntimePort(liveKillPid);
1764-
stopped = true;
1765-
}
1766-
if (stopped) return "stopped";
1767-
if (pid) return "stale";
1768-
return "none";
1572+
await stopProxy(pid);
1573+
removePid(pid);
1574+
removeRuntimePort(pid);
1575+
return "stopped";
17691576
}
17701577

17711578
async function stopTrackedProxyForServiceCommand(): Promise<TrackedProxyCleanupResult> {
@@ -2073,29 +1880,13 @@ export async function serviceCommand(...args: (string | undefined)[]): Promise<v
20731880
ops.start();
20741881
console.log("✅ service started.");
20751882
break;
2076-
case "stop": {
1883+
case "stop":
20771884
assertServiceEnvironmentMatchesInstall();
20781885
// Only stop what is actually installed. The unguarded call ran a real `launchctl unload`
20791886
// (and its Windows/Linux twins) even with nothing installed.
2080-
if (ops.status() !== null || isServiceInstalled()) {
2081-
ops.stop();
2082-
}
1887+
if (ops.status() !== null || isServiceInstalled()) ops.stop();
20831888
await stopTrackedProxyForServiceCommand();
20841889
{
2085-
// Verify rather than trust the stop command: a surviving wrapper respawns its child
2086-
// seconds later, and restoring native Codex on top of a live proxy is the failure #764
2087-
// reports as "stop reports success without stopping the proxy".
2088-
const survivor = await proxyStillLiveAfterStop();
2089-
if (survivor) {
2090-
console.error(
2091-
`❌ service stop did not take effect: a proxy is still listening on port ${survivor.port}.`
2092-
+ "\nNative Codex was NOT restored, because doing so while the proxy is running leaves"
2093-
+ " both pointing at each other. Check for a second service backend (`ocx service status`)"
2094-
+ " or a manually started proxy, then re-run `ocx service stop`.",
2095-
);
2096-
process.exitCode = 1;
2097-
break;
2098-
}
20991890
const restore = restoreNativeCodex();
21001891
if (restore.success) console.log("✅ service stopped + native Codex restored.");
21011892
else console.error(`⚠️ service stopped, but native Codex restore FAILED: ${restore.message}\nRun \`ocx restore\` (or check $CODEX_HOME/config.toml) before using native Codex.`);
@@ -2106,14 +1897,9 @@ export async function serviceCommand(...args: (string | undefined)[]): Promise<v
21061897
else if (!grok.ok) console.error(`⚠️ ${grok.message}`);
21071898
}
21081899
break;
2109-
}
21101900
case "status": {
2111-
if (process.platform === "win32" && backend === "scheduler") {
2112-
console.log(await inspectWindowsSchedulerServiceStatus());
2113-
} else {
2114-
const s = ops.status();
2115-
console.log(s ? `✅ running:\n${s}` : "❌ service not installed/running.");
2116-
}
1901+
const s = ops.status();
1902+
console.log(s ? `✅ running:\n${s}` : "❌ service not installed/running.");
21171903
console.log(`Diagnostics: ${serviceDiagnosticsSummary()}`);
21181904
break;
21191905
}
@@ -2152,4 +1938,3 @@ export async function serviceCommand(...args: (string | undefined)[]): Promise<v
21521938
process.exit(1);
21531939
}
21541940
}
2155-

0 commit comments

Comments
 (0)