Skip to content

Commit d0b44e7

Browse files
committed
merge(dev): sync tip Windows budgets and management-auth fixtures
2 parents ffb9782 + aae9426 commit d0b44e7

4 files changed

Lines changed: 37 additions & 32 deletions

File tree

.github/workflows/ci.yml

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -47,20 +47,31 @@ concurrency:
4747
jobs:
4848
# Which Windows runner this run is allowed to use.
4949
#
50-
# The self-hosted Windows box is a maintainer's PERSONAL machine on a home
51-
# network, reachable over Tailscale. Anything that runs on it runs as a local
52-
# user with that user's filesystem and LAN in reach, so the only question that
53-
# matters is whether the commit is trusted BEFORE a runner ever sees it.
50+
# READ THIS BEFORE TREATING IT AS A SECURITY BOUNDARY: it is not one.
5451
#
55-
# `push` on dev/main/preview is trusted: reaching those branches requires the
56-
# push permission, which only the maintainers in MAINTAINERS.md hold.
57-
# `pull_request` is NOT trusted, and no author check can make it so — the
58-
# workflow file itself comes from the PR head, so any `if:` guard written here
59-
# is deleted by the same patch it is meant to stop. Fork PRs therefore stay on
60-
# GitHub-hosted runners, unconditionally.
52+
# On `pull_request` this workflow is loaded from the PR head, so the `case`
53+
# below is owned by the proposed patch exactly like an `if:` guard would be.
54+
# A hostile PR can delete the branch and hardcode the self-hosted labels into
55+
# `$GITHUB_OUTPUT`, and `runs-on` will honour it. That this job runs on
56+
# `ubuntu-latest` changes nothing — the untrusted part is its OUTPUT, not its
57+
# host. `.github/workflows/ci.yml` is in this workflow's `pull_request.paths`,
58+
# so such an edit triggers its own run.
6159
#
62-
# `workflow_dispatch` is the maintainer escape hatch: dispatch requires write
63-
# access, so a maintainer can deliberately aim a branch at the home box.
60+
# What actually keeps untrusted code off a self-hosted runner lives OUTSIDE
61+
# this file, where a PR cannot reach it: the fork-PR approval policy
62+
# (`all_external_contributors`) and the judgement of whoever clicks approve.
63+
# Runner groups would be the other lever, but they are an organisation
64+
# feature and this repository is user-owned, so the approval policy is the
65+
# only one available here. GitHub's own guidance is to avoid self-hosted
66+
# runners on public repositories for this reason.
67+
#
68+
# So read the routing below as a COST control that keeps honest pull requests
69+
# on GitHub-hosted runners, not as a guarantee about hostile ones.
70+
#
71+
# `push` on dev/main/preview requires the push permission, and
72+
# `workflow_dispatch` requires write access, so both carry a trusted author.
73+
# A trusted author is not audited code: merging a contributor PR into `dev`
74+
# fires `push`, and its dependencies and postinstall hooks then run here.
6475
select-windows-runner:
6576
name: select windows runner
6677
runs-on: ubuntu-latest
@@ -85,10 +96,12 @@ jobs:
8596
push|workflow_dispatch) trusted=yes ;;
8697
esac
8798
88-
# Repository variable OCX_SELF_HOSTED_WINDOWS acts as a kill switch. When
89-
# it is anything other than `1` — including unset, which is the state
90-
# before the runner exists — every job falls back to windows-latest and
91-
# CI keeps working with the box powered off.
99+
# Repository variable OCX_SELF_HOSTED_WINDOWS is an OPERATIONAL switch,
100+
# not a security control: a PR that rewrites this script ignores it for
101+
# the same reason it ignores the event check above. Its job is to keep CI
102+
# working when the box is off or busy. Anything other than `1` —
103+
# including unset, the state before a runner exists — falls back to
104+
# windows-latest.
92105
if [ "$trusted" = "yes" ] && [ "${USE_SELF_HOSTED:-}" = "1" ]; then
93106
echo 'runner=["self-hosted","Windows","X64","ocx-home"]' >> "$GITHUB_OUTPUT"
94107
echo 'label=self-hosted (ocx-home)' >> "$GITHUB_OUTPUT"

tests/server-management-auth.test.ts

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
2+
import { SERVER_BUDGET_MS } from "./helpers/test-budget";
23
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
34
import { tmpdir } from "node:os";
45
import { join } from "node:path";
@@ -446,7 +447,7 @@ describe("management and data-plane credential separation", () => {
446447
} finally {
447448
await server.stop(true);
448449
}
449-
});
450+
}, SERVER_BUDGET_MS); // binds a real server + live fetches; windows runner measured ~5.04s against Bun's 5s default.
450451

451452
test("an existing management token ACL hardening failure keeps management unavailable", async () => {
452453
delete process.env.OPENCODEX_ADMIN_AUTH_TOKEN;
@@ -482,14 +483,7 @@ describe("management and data-plane credential separation", () => {
482483
setPlatformForTests("win32");
483484
setIcaclsRunnerForTests(args => {
484485
const target = args[0] ?? "";
485-
if (target.endsWith("admin-api-token")) {
486-
return { success: true, exitCode: 0, timedOut: false, stdout: "" };
487-
}
488-
// Directory ACE carries (OI)(CI). Timeout only directories so management auth
489-
// fails closed while secret file hardens (config.json.tmp) can still
490-
// succeed — startServer must persist config on real Windows.
491-
const grant = args.find(arg => typeof arg === "string" && arg.includes("(F)")) ?? "";
492-
if (grant.includes("(OI)(CI)")) {
486+
if (target === testHome) {
493487
return { success: false, exitCode: null, timedOut: true, stdout: "" };
494488
}
495489
return { success: true, exitCode: 0, timedOut: false, stdout: "" };
@@ -556,11 +550,9 @@ describe("management and data-plane credential separation", () => {
556550
saveConfig(remoteConfig());
557551
process.env.USERNAME ??= "tester";
558552
setPlatformForTests("win32");
559-
// Stall directory ACL only. Env admin bypasses the file-backed token path;
560-
// secret file hardens for config persistence must still succeed on win32.
561553
setIcaclsRunnerForTests(args => {
562-
const grant = args.find(arg => typeof arg === "string" && arg.includes("(F)")) ?? "";
563-
if (grant.includes("(OI)(CI)")) {
554+
const target = args[0] ?? "";
555+
if (target === testHome || target.endsWith("admin-api-token")) {
564556
return { success: false, exitCode: null, timedOut: true, stdout: "" };
565557
}
566558
return { success: true, exitCode: 0, timedOut: false, stdout: "" };

tests/translator-budget.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
translatorObservedBufferSnapshot,
1111
} from "../src/lib/translator-budget";
1212
import type { AdapterEvent } from "../src/types";
13+
import { SPAWN_BUDGET_MS } from "./helpers/test-budget";
1314

1415
async function textWithin(stream: ReadableStream<Uint8Array>, timeoutMs = 2_000): Promise<string> {
1516
return await Promise.race([
@@ -253,5 +254,4 @@ test("production adapter contract rejects omitted translator budgets at typechec
253254
expect(invalid.stdout.toString() + invalid.stderr.toString()).toContain("TS2554");
254255
const valid = Bun.spawnSync(["bun", ...base, "tests/fixtures/translator-budget-required.valid.ts"]);
255256
expect(valid.exitCode).toBe(0);
256-
// Two bun x tsc --noEmit spawns exceed Bun's 5s default under windows-latest contention.
257-
}, 60_000);
257+
}, SPAWN_BUDGET_MS); // two real tsc child processes ARE the assertion; windows runner measured ~5.5s against Bun's 5s default.

tests/usage-log.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ describe("usage log", () => {
8484
const snapshot = await readUsageSnapshotForManagement();
8585
expect(snapshot.truncatedPrefixBytes).toBeGreaterThan(0);
8686
expect(snapshot.entries.map(entry => entry.requestId)).toEqual(["tail"]);
87-
});
87+
}, STORE_BUDGET_MS); // sparse >64 MiB fixture IO is intrinsic; Windows self-hosted measured 7.193s against Bun's 5s default.
8888

8989
test("usage tail exact row boundary keeps the complete newest row", async () => {
9090
const newest = Buffer.from(`${persistedLine("newest")}\n`);

0 commit comments

Comments
 (0)