Skip to content

[Bug]: Concurrent saveOpenWikiEnv calls can lose credentials or fail during rename #539

Description

Description

saveOpenWikiEnv() performs an unlocked read-modify-write and uses a temporary filename derived only from the process ID. Two calls running concurrently in the same process therefore read the same old state and operate on the same temporary path.

Depending on interleaving, one update can be lost, one call can rename content written by the other, or the second rename can fail with ENOENT after the first call has already moved the shared temp file.

Affected code

src/env.ts:277-333:

  1. line 280 reads the complete current environment;
  2. lines 281-284 merge only the current call's updates;
  3. line 311 chooses ${openWikiEnvPath}.${process.pid}.tmp, identical for every call in the process;
  4. lines 312-317 write, chmod, and rename that shared path.

Atomic rename protects against a crash during one write, but it does not serialize concurrent read-modify-write operations.

Steps to Reproduce

  1. Redirect OpenWiki's home/environment path to a temporary directory as done by test/env-behavior.test.ts.
  2. Start two saves without awaiting either individually:
    await Promise.all([
      saveOpenWikiEnv({ OPENAI_API_KEY: "first" }),
      saveOpenWikiEnv({ OPENROUTER_API_KEY: "second" }),
    ]);
  3. Add a small barrier/mock around filesystem calls if necessary to force both reads to complete before either rename.
  4. Observe one of:
    • only one key is present in the final file;
    • a call fails renaming the already-moved temp path;
    • a call renames bytes produced by the other update.

Expected Behavior

Every successful save should preserve unrelated keys written by overlapping saves, and concurrent callers should not interfere with one another's temporary files.

Actual Behavior

The operation has both a lost-update race and a same-temp-file race.

Why this matters

This file stores API keys, OAuth refresh tokens, model/provider settings, and ngrok callback configuration. Authentication and setup flows are asynchronous, and future parallelization or overlapping UI actions can turn this into credential loss or intermittent login failures.

Suggested direction

  • Protect the full read/merge/write/rename sequence with a process-local mutex.
  • Give every temporary file a unique suffix rather than using PID alone.
  • If cross-process writers are supported, add a lock or compare-and-retry strategy across processes.
  • Keep the existing same-directory atomic rename for crash safety.

Missing regression coverage

The environment tests cover sequential saves and failed atomic replacement, but no test executes disjoint saveOpenWikiEnv() calls concurrently or forces both calls to share an initial read.

Environment

  • OS: macOS
  • Node.js: v24.14.1
  • Affected revision: 63c848c on main
  • OpenWiki version: current source checkout

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions