diff --git a/devlog/_plan/260907_init_publication_guidance/010_implementation.md b/devlog/_plan/260907_init_publication_guidance/010_implementation.md new file mode 100644 index 0000000000..80b5954924 --- /dev/null +++ b/devlog/_plan/260907_init_publication_guidance/010_implementation.md @@ -0,0 +1,28 @@ +# Issue #3893: implementation plan + +Satisfy-spec work, triggered by issue #3893 and the request to implement separate draft PRs. Goal: actionable first-run publication diagnostics. Non-goals: changing file writes, permissions, replacement/cleanup guarantees, or adding a filesystem fallback. Stop after a verified draft PR; unresolved platform checks are reported, never marked passed. Escalate if resolving the issue requires weakening publication guarantees. This file records the plan and eventual evidence. + +Class C2: diagnostic propagation and user documentation. One independent branch from 522ce5f8c; no branch dependencies or orchestration state changes. + +File map: +- MODIFY src/config/initialize.ts: add an optional hardeningFailed flag to constructor options; select a fixed privacy-safe permission diagnostic when hardening throws. Track the flag around the existing harden call only, and pass it in the existing error options. Append supported-location guidance to denied-link diagnostics. All I/O order and cleanup remain identical. +- MODIFY tests/config/config-mutation-lock.test.ts: inject a harden failure and prove write/link never happen, target remains absent, no residue remains, and raw error details do not appear. Assert all five denied-link codes provide recovery guidance while retaining uncertain-publication state. Partial-write errors must not be mislabeled as permission failures. +- MODIFY tests/service/init-eof.test.ts: use its existing child bootstrap seam to inject publication errors during the real CLI wizard; verify exit=1, diagnostics and residue warnings, no configuration/backup damage or integration prompts. +- MODIFY docs-site/src/content/docs/getting-started/quickstart.md and structure/02_config-and-codex-home.md: explain supported locations, inspection before retry, separate permission and link failures, and fresh-install OPENCODEX_HOME examples. Existing translations reviewed for contradictions. + +Optional constructor input chain: created by the publication function; consumed by Error.message; no config serialization, migration, or persistent state. Existing constructor calls keep their meaning. + +Verification: focused config/init tests read the real publication and CLI code; typecheck includes src; privacy scan; required docs-site build. Baseline focused run: 35 pass, 3 skip, 2 fail (Windows file-symlink privilege: symlinkSync EPERM and dependent missing-residue assertion). No baseline failure will be hidden by changing tests. New regression checks must pass. Windows-native filesystem support remains bounded by the host. + +Audit: direct O_EXCL and replacement fallbacks rejected because they change complete-file/no-replace guarantees. Reuse the existing error and test seams; no new diagnostic module. Guidance never prints raw cause text or candidate bytes. + +## Verification before draft publication + +- `bun install --frozen-lockfile`: passed; lockfile unchanged. +- New diagnostics were observed failing before implementation: 9 failures across the focused hardening/link/CLI fault cases. After implementation: 9 passed. +- `bun test tests/config/config-mutation-lock.test.ts tests/service/init-eof.test.ts`: 38 passed, 3 skipped, 2 failed. The same two tests failed on unchanged 522ce5f8c: file-symlink creation is denied on this Windows host, and the swapped-symlink test then lacks its expected residue. New recovery tests pass; no skips or weakened assertions were added. +- `bun run typecheck`: passed. +- `bun run privacy:scan`: passed. +- `cd docs-site; bun install --frozen-lockfile; bun run build`: passed, 425 pages. Translated quickstarts contain no conflicting recovery/fallback instructions. +- CLI fault scenarios verify exit=1, distinct permission/link messages, uncertain-publication/residue warnings, backup preservation and no integration prompts. Partial-write errors keep the generic diagnostic. +- No physical non-NTFS filesystem support is claimed. Maintainer review remains required; this is a draft handoff. diff --git a/devlog/_plan/260908_c_track_config_init_stack/000_plan.md b/devlog/_plan/260908_c_track_config_init_stack/000_plan.md new file mode 100644 index 0000000000..046e2c627d --- /dev/null +++ b/devlog/_plan/260908_c_track_config_init_stack/000_plan.md @@ -0,0 +1,79 @@ +# 000_plan.md — C track: config file + init as a manual PR stack + +## Objective + +Land the C triage track (config-file and init surfaces) on `dev` as one manual, +dependency-ordered branch chain whose tip carries every layer. The track has two +existing contributor pull requests plus one gap discovered while planning: + +| Layer | Source | Surface | +|---|---|---| +| wp1 | PR #3900 by @x3M3x | `src/config/atomic-write.ts` Bun/Windows ENOENT | +| wp2 | new (this unit) | `src/config/initialize.ts` sibling numeric flag | +| wp3 | PR #3896 by @parkjs101 | `ocx init` publication recovery guidance (closes #3893) | + +wp2 exists on its own merit, not as glue. `publishInitialConfigNoReplace` still +opens its temp file with the numeric spelling that Bun miscompiles on Windows, +so shipping wp1 alone leaves first-run config publication exposed to the same +`ENOENT`. #3900 never touches `initialize.ts`; the file overlap is between wp2 +and wp3 only. + +## Constraints (owner-stated, this session) + +- **No local product suite.** No `bun run test`, `bun run typecheck`, + `bun run build`, or install. Every such check is recorded **NOT RUN**. +- **Push with `--no-verify`** on every layer. +- **CI on the tip only.** Verified mechanism in `040`: the lower layers are + pushed as branches but **no pull request is opened for them** until the tip + has landed. `.github/workflows/ci.yml` triggers on `pull_request: {}` with no + draft filter, so opening a lower PR would start CI; draft status suppresses + nothing. +- **Green tip merges; the rest resolve.** When the tip's exact head SHA is green + against a current `dev` base, merge the tip, then resolve the source PRs and + close issue #3893. +- **Original authors are preserved** with full `Co-authored-by: Name ` + trailers that survive the squash (AGENTS.md "Landing another author's work"). + +## Build order + +``` +codex/c-track-init-guidance → the ONLY pull request (base dev) ← wp3 tip +codex/c-track-initialize-flag → branch only, no PR ← wp2 +codex/c-track-atomic-write → branch only, no PR ← wp1 +──────────────────────────────── dev +``` + +Each branch is based on the one below, so the tip's tree is the cumulative +result. wp2 sits between the carried PRs because wp3 inserts a line directly +after the `openSync` call that wp2 rewrites; constructing wp2 first means that +adjacent-hunk overlap is resolved once while carrying wp3. This is a chosen +construction order for a single conflict resolution, not a semantic +prerequisite — either change could be written first. + +## Scope boundary + +IN: the three layers above, their regression tests, the docs/structure text that +#3896 already carries, and this devlog unit. + +OUT: `#3838`/`#3917` adapter work, any other triage track, release promotion, +`main`/`preview`, and any behavioral change to hard-link publication, ACL +hardening, or credential storage beyond the flag spelling. + +## Verifiers + +Local product gates are forbidden this session, so acceptance rests on +repository CI against the tip plus read-only inspection. + +| Claim | Evidence | Status | +|---|---|---| +| Layers carry original authorship | `git log --format='%(trailers:key=Co-authored-by)'` on the tip, then on the landed commit | to run (read-only) | +| wp2 removes the numeric spelling | `rg 'constants\.O_' src/config/initialize.ts` on the pushed tip tree | to run (read-only) | +| Carried content really landed | tip tree vs. each source PR's pinned patch, then landed-merge tree comparison (see `040`) | to run (read-only) | +| Layers build and pass | repository CI on the tip head SHA, base `dev` | tip only | +| Local suite / typecheck / build | — | **NOT RUN** (owner instruction) | + +## Terminal outcome + +DONE requires: tip CI success on its exact head SHA against a current `dev` +base, tip merged into `dev` proven by fetched ancestry and tree comparison, +source PRs resolved with credit intact, and #3893 closed. diff --git a/devlog/_plan/260908_c_track_config_init_stack/010_layer1_atomic_write.md b/devlog/_plan/260908_c_track_config_init_stack/010_layer1_atomic_write.md new file mode 100644 index 0000000000..d9e76ba2bb --- /dev/null +++ b/devlog/_plan/260908_c_track_config_init_stack/010_layer1_atomic_write.md @@ -0,0 +1,72 @@ +# 010_layer1_atomic_write.md — wp1: carry PR #3900 + +Source: [PR #3900](https://github.com/lidge-jun/opencodex/pull/3900) by @x3M3x, +head `744eb644028492784446fe9f0f73813d5d1fe59f`, two commits +(`52c749561`, `744eb6440`). + +Branch: `codex/c-track-atomic-write`, base `dev`. + +## Problem + +`src/config/atomic-write.ts` builds its exclusive-create flags numerically. +Bun on Windows misreads that combination and drops the creation bit, so every +private temp write fails with `ENOENT`: `ocx start`, management-API config +saves, and OAuth credential refreshes all route through these two writers. + +## Change (MODIFY, carried unmodified from #3900) + +`src/config/atomic-write.ts` + +```diff +- const descriptor = openSync(path, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL, 0o600); ++ const descriptor = openSync(path, "wx", 0o600); +``` + +Applied in both `writePrivateTempFile` and `writePrivateTempFileAsync`; the now +unused `constants` import is dropped. + +`tests/windows/windows-secret-acl.test.ts` gains the source-oracle guard +"atomic secret temp writer portability", asserting exactly two portable calls. + +## Semantics note (audit correction) + +The PR description calls `"wx"` exactly `O_WRONLY | O_CREAT | O_EXCL`. Node and +Bun actually map it to `O_WRONLY | O_CREAT | O_EXCL | O_TRUNC`. It is +**behaviorally** equivalent here rather than bit-identical: exclusive creation +rejects an existing path, so `O_TRUNC` can never truncate one. `0o600` remains +a separate mode argument and still applies. Recorded so a later reader does not +inherit the imprecise claim. + +## Security review (independent, read-only) + +No blocking finding. Exclusivity is preserved for every caller of the default +wrappers, which include OAuth `auth.json`, Codex account credentials, service +API tokens, `config.json`, and `ocx.pid`. Ownership is marked only after a +successful create, so no new pre-existing-temp or symlink-following path opens. +Windows ACL ordering (create → own → harden → identity check → write → close) +is untouched. + +## Authorship + +Both commits are cherry-picked with `-x`, so each retains +`x3M3x ` as its git author and records the source SHA: + +| Carried commit | Source commit | +|---|---| +| `6a0abcf90` fix: use portable exclusive config temp creation | `52c7495618f18f2847b7f9468421442c1c573da1` | +| `24a078d80` test: guard atomic temp writes against Bun/Windows ENOENT | `744eb644028492784446fe9f0f73813d5d1fe59f` | + +A squash landing keeps only the squash message and drops per-commit authors, so +`b1a7f111c` adds the `Co-authored-by: x3M3x ` trailer to +the branch. That trailer must be carried into the tip PR squash message and +re-read on the landed commit. + +## Verification + +An independent read-only audit of the built branch confirmed the carried +source-and-test diff is byte-identical to #3900 pinned patch (2,176 bytes), +that both `-x` annotations and the original author survive, that the trailer +parses through `git interpret-trailers`, and that no other `src/` file changed. + +Repository CI on the stack tip only. Local suite, typecheck, and build: +**NOT RUN** (owner instruction). diff --git a/devlog/_plan/260908_c_track_config_init_stack/020_layer2_initialize_flag.md b/devlog/_plan/260908_c_track_config_init_stack/020_layer2_initialize_flag.md new file mode 100644 index 0000000000..b25a419481 --- /dev/null +++ b/devlog/_plan/260908_c_track_config_init_stack/020_layer2_initialize_flag.md @@ -0,0 +1,69 @@ +# 020_layer2_initialize_flag.md — wp2: sibling flag in initialize.ts + +New work in this unit. Branch `codex/c-track-initialize-flag`, base +`codex/c-track-atomic-write`. + +## Why this layer exists + +wp1 fixes the two writers in `atomic-write.ts`, but +`publishInitialConfigNoReplace` in `src/config/initialize.ts` still opens its +temp file with the same numeric spelling. Independent inspection confirms the +identical Bun/Windows exposure: first-run `ocx init` fails before writing or +publishing `config.json`, leaving `publication = "not-published"` and +`hardLinkUnavailable = false`, so the CLI prints "Initial config publication +did not finish." and exits 1. + +It also sits between the two carried PRs deliberately. #3896 inserts a line +immediately after this `openSync` call, so building wp2 first means the +adjacent-hunk overlap is resolved once, while carrying #3896 in wp3. This is a +chosen construction order rather than a semantic prerequisite: #3900 does not +touch this file at all, and either change could be written first. + +## Change (MODIFY) + +`src/config/initialize.ts` + +```diff + import { +- closeSync, constants, fchmodSync, fstatSync, linkSync, lstatSync, ++ closeSync, fchmodSync, fstatSync, linkSync, lstatSync, + openSync, unlinkSync, writeFileSync, + } from "node:fs"; +@@ +- fd = openSync(temp, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL, 0o600); ++ fd = openSync(temp, "wx", 0o600); +``` + +`constants` is referenced only by the import and that one expression, so the +import entry is removed with it. + +## Change (MODIFY, regression) + +`tests/windows/windows-secret-acl.test.ts` gains a sibling source-oracle test +next to the wp1 guard, asserting exactly one portable call in +`src/config/initialize.ts`. + +## Contract preserved + +The no-replace publication contract does not depend on the numeric spelling: +hardening, `verifyPrivateTemp`, the single `linkSync` publication with its +`EEXIST`/`collided` and `hardLinkUnavailable` handling, and the +descriptor-owned `removeOwnedTemp` cleanup are all unchanged. + +## Out of scope: the same pattern under `src/lab/` + +An independent scan found three more exclusive opens sharing this combination: +`src/lab/ledger/store.ts:153` and `:185` (recovery mutex, ledger lock) and +`src/lab/public/private-file.ts:209` (private publication temp). They deserve the +same portability follow-up, but Lab is an opt-in subsystem off the core request +path, so they stay out of this track rather than widening a config-surface fix. + +Two further matches are not exclusive opens and must not be swept in: +`src/codex/native-main-lock-file.ts:89` and `src/lab/fabric/scratch.ts:416`. The +read/write sites in `src/lab/artifacts/secure-fs.ts` need individual treatment because +`"wx"` would drop read access. + +## Verification + +Repository CI on the stack tip only. Local suite, typecheck, and build: +**NOT RUN** (owner instruction). diff --git a/devlog/_plan/260908_c_track_config_init_stack/030_layer3_init_guidance.md b/devlog/_plan/260908_c_track_config_init_stack/030_layer3_init_guidance.md new file mode 100644 index 0000000000..5f755c686d --- /dev/null +++ b/devlog/_plan/260908_c_track_config_init_stack/030_layer3_init_guidance.md @@ -0,0 +1,65 @@ +# 030_layer3_init_guidance.md — wp3: carry PR #3896 (stack tip) + +Source: [PR #3896](https://github.com/lidge-jun/opencodex/pull/3896) by +@parkjs101 (Joonsuh Park), head `fc78bc37d419576061b995281baf39c46655eaa9`, +closes [#3893](https://github.com/lidge-jun/opencodex/issues/3893). + +Branch: `codex/c-track-init-guidance`, based on `codex/c-track-initialize-flag`. +This is the stack tip and the only pull request opened for this unit. + +## Problem + +`ocx init` already separates denied hard-link publication from a generic +failure, but a required permission-hardening failure falls into the generic +message, and neither message tells the user what to do next. The user cannot +tell why publication stopped or where to move `OPENCODEX_HOME`. + +## Change (carried from #3896, rebased onto wp2) + +- `src/config/initialize.ts`: a `hardeningFailed` flag set immediately before + the hardening call and cleared immediately after it returns — the assignments + surround `hardenInitialConfig`, which runs after `openSync`. The flag selects + a distinct message; `InitialConfigPublicationError` takes the matching option, + and both messages gain `OPENCODEX_HOME` recovery direction. +- `docs-site/src/content/docs/getting-started/quickstart.md`: inspection before + retry, preserving existing configuration, choosing a supported location. +- `structure/02_config-and-codex-home.md`: records the diagnostic distinction. +- `tests/config/config-mutation-lock.test.ts`, `tests/service/init-eof.test.ts`: + permission, link, and cleanup faults, privacy-safe messages, backup + preservation. Both files already exist in the test-layout registries, so no + registry entry is added. +- `devlog/_plan/260907_init_publication_guidance/010_implementation.md`: carried + as-is; on terminal closure that unit moves to `_fin/`. + +The rebase keeps wp2's `openSync(temp, "wx", 0o600)` and both `hardeningFailed` +assignments around the hardening call. + +## Review (independent subagent audit, read-only, this session) + +Reviewed at `fc78bc37d`, all six files. No blocking finding: + +- The flag cannot be left incorrectly true. Write, verify, link, and close + failures all occur after it is cleared (`src/config/initialize.ts:95-115`, + `:129-134`). A throwing injected `io.harden` test seam would select the same + message, which is a seam edge rather than a production defect. +- File I/O ordering, the no-replace guarantee, and private permissions are + unchanged (`:38-43`, `:98-130`). +- The new messages are fixed text naming the `OPENCODEX_HOME` variable; they + interpolate no real paths, bytes, or filesystem error text, and raw errors + stay in the `cause` the CLI does not print (`src/cli/init.ts:262-267`). + +This is a static agent review, not the maintainer security review or the +approval required by `MAINTAINERS.md`; those are recorded separately in `040`. + +## Authorship + +Carried with `Co-authored-by: Joonsuh Park `, the identity +on the source commit. The trailer must appear in the **squash message** of the +landed commit, and is verified on the landed commit rather than only on the +branch. + +## Verification + +This tip is the only layer that triggers repository CI, and its exact head SHA +must be green against a current `dev` base. Local suite, typecheck, and build: +**NOT RUN** (owner instruction). diff --git a/devlog/_plan/260908_c_track_config_init_stack/040_layer4_landing.md b/devlog/_plan/260908_c_track_config_init_stack/040_layer4_landing.md new file mode 100644 index 0000000000..4413166db9 --- /dev/null +++ b/devlog/_plan/260908_c_track_config_init_stack/040_layer4_landing.md @@ -0,0 +1,62 @@ +# 040_layer4_landing.md — wp4: tip CI, merge, and closure + +## CI suppression: mechanism, not draft status + +`.github/workflows/ci.yml` triggers on `pull_request: {}` with **no draft +filter** (line 7), and the `changes` job gates expensive work on the PR's own +file list, which includes `src/**` and `tests/**`. Opening a lower-layer pull +request — draft or not — therefore starts repository CI. + +The mechanism that actually satisfies "CI on the tip only" is to **open no pull +request for wp1 and wp2**. Their branches are pushed so the tip has a real +parent chain, but only `codex/c-track-init-guidance` gets a PR, based directly +on `dev` so its diff is the cumulative stack. One workflow run, one subject. + +## Landing sequence + +1. Push all three branches with `--no-verify`. +2. Open the tip PR only, base `dev`, with the full repository template + (Summary, Verification, Checklist) and `Closes #3893`. Record the local + suite as NOT RUN with the owner instruction as the reason; the Verification + section must not imply a local green run. +3. Confirm the tip is based on the current `dev` head before CI. If `dev` has + advanced, rebase and cascade first — CI against a stale base does not certify + the integration tree that will actually merge. +4. Wait for CI on the tip's exact head SHA. Skipped or cancelled checks are not + passing evidence. +5. Record the merge decision. Both current maintainers hold `admin`, and + `MAINTAINERS.md` permits explicit maintainer integration into `dev` without a + second approval, provided the decision and exact-head CI evidence are + recorded and security review is kept separate. The credential-adjacent + `atomic-write.ts` carry is the security-review subject; its independent audit + is summarized in `010` and must be named in the merge record. +6. Merge the tip, pinning the reviewed head SHA. + +## Proving the carried work landed + +Ancestry alone is insufficient: `dev` can contain the merge while a conflict +resolution silently dropped a contributor hunk. Before closing anything: + +- Compare each source PR's pinned patch against the tip tree, documenting the + one intentional adaptation (wp2 rewrites the `openSync` line that #3896's + hunk sits next to). +- After the merge, compare the landed tree on fetched `dev` against the + reviewed tip tree. +- Re-read the landed commit's trailers to confirm both `Co-authored-by` entries + survived the squash. + +A squash landing does not make the original contributor SHAs ancestors, so +trailer and content comparison are the credit and delivery evidence. + +## Closure + +1. Close #3900 and #3896 as landed through the tip, naming the merge commit and + crediting @x3M3x and @parkjs101 with the evidence above. +2. Close issue #3893: PRs here target `dev`, and GitHub only auto-closes linked + issues on the default branch. + +## Failure handling + +If the tip's CI fails, fix the responsible layer and cascade the rebase upward +(`DEV-STACK-02`) before re-running CI on the new tip head. Do not open or merge +a lower layer independently to bypass a red tip. diff --git a/docs-site/src/content/docs/getting-started/quickstart.md b/docs-site/src/content/docs/getting-started/quickstart.md index 1fdcf922d8..867a06cec1 100644 --- a/docs-site/src/content/docs/getting-started/quickstart.md +++ b/docs-site/src/content/docs/getting-started/quickstart.md @@ -36,6 +36,34 @@ stop setup without falling back to an overwrite. If publication or temporary-fil finish, inspect the config directory before retrying: a complete config or private temporary file may remain. +If setup reports that initial config permissions could not be secured, the filesystem or account +could not apply the required private permissions (NTFS ACLs on Windows). This happens before +config contents are written. A hard-link publication error is a separate failure: private +permissions were applied, but publishing the completed file failed or its outcome is uncertain. + +Inspect the selected config directory before retrying. Preserve any existing `config.json`; +do not delete it to force setup to proceed. For a fresh installation, choose a writable location +that supports both hard links and private permissions. A local NTFS directory is a suitable +Windows choice when your account can apply its ACLs. For example, select a new location in the +same terminal before running setup: + +```powershell +# Windows PowerShell: choose a fresh directory on a local NTFS volume. +$env:OPENCODEX_HOME = Join-Path $env:LOCALAPPDATA "opencodex-local" +ocx init +``` + +```sh +# macOS/Linux: choose a fresh directory on a filesystem with hard links and Unix permissions. +export OPENCODEX_HOME="$HOME/.opencodex-local" +ocx init +``` + +Use the same `OPENCODEX_HOME` for subsequent commands and the service that runs the proxy. +Changing this variable selects a separate configuration location; it does not migrate an existing +installation. Setup intentionally has no direct-write or replacing-rename fallback: creating an +exclusive file and then writing to it could expose partial config contents. + :::note[GPT-5.6 rollout entries] The current stable release seeds GPT-5.6 Sol/Terra/Luna for ChatGPT passthrough, OpenAI API-key, OpenRouter, and diff --git a/src/config/atomic-write.ts b/src/config/atomic-write.ts index 0ec0831c4c..69bc112146 100644 --- a/src/config/atomic-write.ts +++ b/src/config/atomic-write.ts @@ -1,7 +1,6 @@ import { chmodSync, closeSync, - constants, fchmodSync, fstatSync, lstatSync, @@ -121,7 +120,7 @@ function writePrivateTempFile( timeoutMemoKey: string, onCreated: () => void, ): void { - const descriptor = openSync(path, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL, 0o600); + const descriptor = openSync(path, "wx", 0o600); onCreated(); try { if (process.platform === "win32") { @@ -142,7 +141,7 @@ async function writePrivateTempFileAsync( timeoutMemoKey: string, onCreated: () => void, ): Promise { - const descriptor = openSync(path, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL, 0o600); + const descriptor = openSync(path, "wx", 0o600); onCreated(); try { if (process.platform === "win32") { diff --git a/src/config/initialize.ts b/src/config/initialize.ts index 864b09b036..ed9d121eaa 100644 --- a/src/config/initialize.ts +++ b/src/config/initialize.ts @@ -1,5 +1,5 @@ import { - closeSync, constants, fchmodSync, fstatSync, linkSync, lstatSync, + closeSync, fchmodSync, fstatSync, linkSync, lstatSync, openSync, unlinkSync, writeFileSync, } from "node:fs"; import { dirname } from "node:path"; @@ -15,10 +15,12 @@ export class InitialConfigPublicationError extends Error { readonly publication: PublicationState, readonly residualTemp: boolean, readonly hardLinkUnavailable: boolean, - options?: ErrorOptions, + options?: ErrorOptions & { hardeningFailed?: boolean }, ) { - super(hardLinkUnavailable - ? "Initial config requires hard-link publication; the filesystem or its permissions denied it." + super(options?.hardeningFailed + ? "Initial config permissions could not be secured. Choose an OPENCODEX_HOME location that supports private file permissions (NTFS ACLs on Windows), then rerun `ocx init`." + : hardLinkUnavailable + ? "Initial config requires hard-link publication; the filesystem or its permissions denied it. Inspect the config directory before retrying. Choose an OPENCODEX_HOME location that supports hard links and private file permissions, then rerun `ocx init`." : "Initial config publication did not finish.", options); this.name = "InitialConfigPublicationError"; } @@ -90,10 +92,13 @@ export function publishInitialConfigNoReplace( let failure: unknown; let failed = false; let hardLinkUnavailable = false; + let hardeningFailed = false; let residualTemp = false; try { - fd = openSync(temp, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL, 0o600); + fd = openSync(temp, "wx", 0o600); + hardeningFailed = true; (io.harden ?? hardenInitialConfig)(fd, temp, target); + hardeningFailed = false; verifyPrivateTemp(fd, temp); (io.write ?? ((descriptor: number, value: string) => writeFileSync(descriptor, value, { encoding: "utf8" })))(fd, bytes); verifyPrivateTemp(fd, temp); @@ -126,7 +131,7 @@ export function publishInitialConfigNoReplace( } } if (failed || residualTemp) { - throw new InitialConfigPublicationError(publication, residualTemp, hardLinkUnavailable, { cause: failure }); + throw new InitialConfigPublicationError(publication, residualTemp, hardLinkUnavailable, { cause: failure, hardeningFailed }); } return !collided; } diff --git a/structure/02_config-and-codex-home.md b/structure/02_config-and-codex-home.md index 9478343d19..572c73f25f 100644 --- a/structure/02_config-and-codex-home.md +++ b/structure/02_config-and-codex-home.md @@ -284,6 +284,12 @@ and publication followed by a later failure can leave a complete config or priva foreign winner's ownership under future uninstall; the existing ownership manifest and global CLI shim preflight keep their separate contracts. +Initial publication diagnostics distinguish required permission-hardening failures from denied +hard-link publication without exposing raw filesystem causes. Both identify `OPENCODEX_HOME` +as the supported-location recovery path; uncertain publication and cleanup warnings remain in +the CLI. The quickstart documents inspection before retry, private-permission requirements, +and fresh-location examples. Diagnostics do not introduce a fallback or alter file I/O ordering. + `src/config/paths.ts` is the single owner of `OPENCODEX_HOME` expansion and resolution. It exposes the config directory and `config.json` path and retains the existing cache rule: a relative home is resolved once for each distinct raw environment value, so a later working-directory change cannot diff --git a/tests/config/config-mutation-lock.test.ts b/tests/config/config-mutation-lock.test.ts index 06a18dd99d..56a4d74659 100644 --- a/tests/config/config-mutation-lock.test.ts +++ b/tests/config/config-mutation-lock.test.ts @@ -230,22 +230,38 @@ test("exclusive temp collision does not remove or modify somebody else's file", test("failed hardening occurs before candidate bytes are written", () => { let wrote = false; - expect(() => initializePersistedConfigIfMissing(config(), { - harden(_fd, temp) { - expect(readFileSync(temp, "utf8")).toBe(""); - throw new Error("ACL denied"); - }, - write() { wrote = true; }, - })).toThrow(InitialConfigPublicationError); + let linked = false; + let failure: unknown; + try { + initializePersistedConfigIfMissing(config(), { + harden(_fd, temp) { + expect(readFileSync(temp, "utf8")).toBe(""); + throw new Error("private ACL failure detail"); + }, + write() { wrote = true; }, + link() { linked = true; }, + }); + } catch (error) { failure = error; } + expect(failure).toBeInstanceOf(InitialConfigPublicationError); + expect((failure as Error).message).toContain("permissions could not be secured"); + expect((failure as Error).message).toContain("OPENCODEX_HOME"); + expect((failure as Error).message).not.toContain("private ACL failure detail"); + expect(failure).toMatchObject({ publication: "not-published", hardLinkUnavailable: false, residualTemp: false }); expect(wrote).toBe(false); + expect(linked).toBe(false); expect(existsSync(getConfigPath())).toBe(false); expect(initTemps()).toEqual([]); }); test("partial write failure removes only the unpublished temporary name", () => { - expect(() => initializePersistedConfigIfMissing(config(), { - write(fd, bytes) { writeFileSync(fd, bytes.slice(0, 10)); throw new Error("disk full"); }, - })).toThrow(InitialConfigPublicationError); + let failure: unknown; + try { + initializePersistedConfigIfMissing(config(), { + write(fd, bytes) { writeFileSync(fd, bytes.slice(0, 10)); throw new Error("disk full"); }, + }); + } catch (error) { failure = error; } + expect(failure).toBeInstanceOf(InitialConfigPublicationError); + expect((failure as Error).message).toBe("Initial config publication did not finish."); expect(existsSync(getConfigPath())).toBe(false); expect(initTemps()).toEqual([]); }); @@ -259,6 +275,10 @@ test.each(["EOPNOTSUPP", "ENOTSUP", "ENOSYS", "EXDEV", "EPERM"])("unsupported/de } catch (error) { expect(error).toBeInstanceOf(InitialConfigPublicationError); expect((error as InitialConfigPublicationError).hardLinkUnavailable).toBe(true); + expect((error as Error).message).toContain("OPENCODEX_HOME"); + expect((error as Error).message).toContain("private file permissions"); + expect((error as Error).message).not.toContain("do not print raw error"); + expect((error as Error).message).not.toContain("permissions could not be secured"); } expect(existsSync(getConfigPath())).toBe(false); expect(initTemps()).toEqual([]); diff --git a/tests/service/init-eof.test.ts b/tests/service/init-eof.test.ts index e098e5c56c..df863f13af 100644 --- a/tests/service/init-eof.test.ts +++ b/tests/service/init-eof.test.ts @@ -173,6 +173,55 @@ describe("ocx init piped stdin (#754)", () => { } finally { await stop(proc); } }, 30_000); + test.each(["permissions", "link", "link-residue"])("publication recovery guidance reaches the CLI (%s)", async failure => { + const home = makeHome(); + const backup = join(home, "config.json.pre-openai-tiers-v2.bak"); + writeFileSync(backup, "preserve backup on publication failure"); + const bootstrap = ` + import { mock } from "bun:test"; + const configApi = { ...await import("./src/config.ts") }; + const failure = ${JSON.stringify(failure)}; + const io = failure === "permissions" + ? { harden() { throw new Error("private permission detail"); } } + : { + link() { throw Object.assign(new Error("private link detail"), { code: "EPERM" }); }, + ...(failure === "link-residue" ? { unlink() { throw new Error("private cleanup detail"); } } : {}), + }; + mock.module("./src/config.ts", () => ({ + ...configApi, + initializePersistedConfigIfMissing(config) { + return configApi.initializePersistedConfigIfMissing(config, io); + }, + })); + const { runInit } = await import("./src/cli/init.ts"); + await runInit(); + `; + const proc = launch(home, "init", bootstrap); + const stderr = new Response(proc.stderr).text(); + try { + await reachPortPrompt(proc); + proc.stdin.write("21001\n"); + await proc.stdin.flush(); + const stdout = remainingOutput(proc.stdout); + expect(await proc.exited).toBe(1); + const diagnostic = await stderr; + expect(diagnostic).toContain("OPENCODEX_HOME"); + expect(diagnostic).toContain("ocx init"); + expect(diagnostic).not.toMatch(/fixture-init-key|private (permission|link|cleanup) detail/); + if (failure === "permissions") { + expect(diagnostic).toContain("permissions could not be secured"); + expect(diagnostic).not.toContain("Config may already exist"); + } else { + expect(diagnostic).toContain("hard-link publication"); + expect(diagnostic).toContain("Config may already exist; inspect it before retrying"); + } + expect(diagnostic.includes("A temporary file could not be removed")).toBe(failure === "link-residue"); + expect(await stdout).not.toMatch(/Inject into|autostart shim|Setup complete/); + expect(existsSync(join(home, "config.json"))).toBe(false); + expect(readFileSync(backup, "utf8")).toBe("preserve backup on publication failure"); + } finally { await stop(proc); } + }, 30_000); + // Windows process.kill does not deliver a POSIX SIGINT to readline. test.skipIf(process.platform === "win32")("SIGINT settles a pending prompt without creating config", async () => { const home = makeHome(); diff --git a/tests/windows/windows-secret-acl.test.ts b/tests/windows/windows-secret-acl.test.ts index aa011bb516..29d1b7bc3f 100644 --- a/tests/windows/windows-secret-acl.test.ts +++ b/tests/windows/windows-secret-acl.test.ts @@ -632,6 +632,30 @@ describe("icacls executable authority", () => { }); }); +describe("atomic secret temp writer portability", () => { + test("sync and async secret temp writers use Bun-portable exclusive creation", async () => { + // Bun on Windows misinterpreted the equivalent numeric O_* combination as + // ENOENT, so every pid/config/oauth temp write failed during ocx start + // and on management-API config saves. Keep both writers on the portable + // exclusive-write spelling ("wx" keeps O_EXCL; 0o600 keeps the private + // mode) so the O_CREAT bit can never be dropped again. + const src = readFileSync(repoPath("src", "config", "atomic-write.ts"), "utf8"); + expect(src.match(/openSync\(path, "wx", 0o600\)/g)).toHaveLength(2); + }); +}); + +describe("initial config temp writer portability", () => { + test("initial config publication uses Bun-portable exclusive creation", () => { + // publishInitialConfigNoReplace carries the same Bun/Windows exposure as the + // atomic writers above: the numeric O_* combination lost its creation bit, so + // first-run `ocx init` failed before it could publish config.json. Exclusive + // creation is what makes the added O_TRUNC harmless — an existing temp name + // (or a symlink planted at one) fails the open instead of being truncated. + const src = readFileSync(repoPath("src", "config", "initialize.ts"), "utf8"); + expect(src.match(/openSync\(temp, "wx", 0o600\)/g)).toHaveLength(1); + }); +}); + describe("diagnostics sanitization contract", () => { test("HardenResult diagnostics field is a plain string when present", () => { const filePath = join(testDir, "diag-test.json");