Conversation
🔒 Aisle Security AnalysisWe found 2 potential security issue(s) in this PR:
1. 🔵 Race condition in async installGaxiosFetchCompat can allow requests before proxy/TLS dispatcher patch is applied
DescriptionThe async initializer Because the function now performs asynchronous work (dynamic module resolution/import), concurrent callers can observe
This creates a startup-time window where requests can bypass expected proxy/TLS routing controls. Vulnerable code: export async function installGaxiosFetchCompat(): Promise<void> {
if (installState !== "not-installed" || typeof globalThis.fetch !== "function") {
return;
}
installState = "installing";
// async work ...
}Notes on current repo call sites:
RecommendationMake installation idempotent and awaitable by sharing a single in-flight promise. Callers arriving during Example fix: let installPromise: Promise<void> | null = null;
export function installGaxiosFetchCompat(): Promise<void> {
if (typeof globalThis.fetch !== "function") return Promise.resolve();
if (installState === "installed" || installState === "shimmed") return Promise.resolve();
if (installState === "installing" && installPromise) return installPromise;
installState = "installing";
installPromise = (async () => {
const Gaxios = await loadGaxiosConstructor();
if (!Gaxios) {
installLegacyWindowFetchShim();
installState = "shimmed";
return;
}
const prototype = Gaxios.prototype;
const originalDefaultAdapter = prototype._defaultAdapter;
const compatFetch = createGaxiosCompatFetch();
prototype._defaultAdapter = function patchedDefaultAdapter(this: unknown, config: GaxiosFetchRequestInit) {
return originalDefaultAdapter.call(this, config.fetchImplementation ? config : {
...config,
fetchImplementation: compatFetch,
});
};
installState = "installed";
})().catch(err => {
installState = "not-installed";
throw err;
});
return installPromise;
}Optionally, consider whether 2. 🔵 CI smoke test runs
|
| Property | Value |
|---|---|
| Severity | Low |
| CWE | CWE-693 |
| Location | .github/workflows/ci.yml:322-326 |
Description
The new CI smoke test step runs node openclaw.mjs status --json --timeout 1 on PRs. The status --json command performs multiple network-capable operations that are not disabled by --timeout 1:
- NPM registry HTTP request:
getUpdateCheckResult({ includeRegistry: true })callsfetchWithTimeout("https://registry.npmjs.org/openclaw/<tag>"). - Git remote fetch:
getUpdateCheckResult({ fetchGit: true })runsgit fetch --quiet --prune. - Gateway probe WebSocket connection attempt:
scanStatusJsonFast()callsprobeGateway({ url, timeoutMs }), which creates aGatewayClientconnection to the resolved Gateway URL.
Even with --timeout 1, the gateway probe enforces a minimum 250ms timer, and the update check uses a fixed ~2500ms timeout (independent of CLI --timeout).
Security impact / risk in CI context:
- Outbound requests from PR jobs can be undesirable (policy/compliance) and may leak metadata (runner IP, timing) to external services.
- Adds flakiness due to reliance on network availability.
Relevant code locations:
- Workflow step:
.github/workflows/ci.yml - Status JSON scan triggers update check + probe:
src/commands/status.scan.fast-json.ts(getUpdateCheckResult({ fetchGit: true, includeRegistry: true }),resolveGatewayProbeSnapshot()) - Registry fetch + git fetch implementation:
src/infra/update-check.ts(fetchWithTimeout("https://registry.npmjs.org/... "),git fetch --quiet --prune) - Gateway probe uses WebSocket client + min timer:
src/gateway/probe.ts(Math.max(250, opts.timeoutMs)timer)
Recommendation
Make the CI smoke test deterministic and offline-safe:
- Prefer a pure local smoke test that does not reach the network (e.g., only
--help, or a dedicated--smoke/--offlinemode). - If you need to keep
status --jsonin CI, add and use a flag/env to disable update checks and probes in smoke contexts (example):
- name: Smoke test CLI launcher status json (offline)
env:
OPENCLAW_DISABLE_UPDATE_CHECK: "1"
OPENCLAW_DISABLE_GATEWAY_PROBE: "1"
run: node openclaw.mjs status --json --timeout 1and gate the implementation in code (example):
const disableUpdate = process.env.OPENCLAW_DISABLE_UPDATE_CHECK === "1";
const updatePromise = disableUpdate
? Promise.resolve({ /* minimal update object */ })
: getUpdateCheckResult({ timeoutMs: updateTimeoutMs, fetchGit: true, includeRegistry: true });This keeps CI from making outbound HTTP/git requests and reduces flakiness.
Analyzed PR: #48501 at commit be78da9
Last updated on: 2026-03-16T23:00:41Z
openclaw-barnacle
bot
added
docs
Takhoffman
force-pushed
the
codex/startup-memory-launcher-fix
branch
from
aa81801 to
b53818c
Compare
openclaw-barnacle
bot
added
size: M
and removed
docs
Greptile SummaryThis PR fixes two startup regressions: (1) the
Confidence Score: 4/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/infra/gaxios-fetch-compat.ts
Line: 228-246
Comment:
**Async gap leaves idempotency guard open to concurrent calls**
`installState` is checked before the `await loadGaxiosConstructor()` call but is not updated until after the `await` resolves. If two callers invoke `installGaxiosFetchCompat()` concurrently (both are awaited on separate microtask queues before either resolves), both will pass the `installState !== "not-installed"` guard simultaneously. The first caller then patches `Gaxios.prototype._defaultAdapter` and sets `installState = "installed"`, but the second caller has already captured `originalDefaultAdapter` before the first patched it — or the second call proceeds and double-wraps the adapter.
In practice this is low risk today (both call sites are serial), but the invariant is fragile. A minimal fix is to set `installState` to a sentinel before the `await`:
```typescript
export async function installGaxiosFetchCompat(): Promise<void> {
if (installState !== "not-installed" || typeof globalThis.fetch !== "function") {
return;
}
installState = "installed"; // claim the slot before the first await
const Gaxios = await loadGaxiosConstructor();
if (!Gaxios) {
installLegacyWindowFetchShim();
installState = "shimmed";
return;
}
// ... rest of install
}
```
(Adjust the sentinel value as appropriate for the final state machine.)
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: test/openclaw-launcher.e2e.test.ts
Line: 7-16
Comment:
**Temp directory leaks if `makeLauncherFixture` throws mid-setup**
`fs.mkdtemp` creates the directory before `fs.copyFile` and `fs.mkdir` are called. If either of those subsequent operations throws, the function exits without returning the path, so the caller never pushes it to `fixtureRoots`, and the `afterEach` cleanup never runs on it.
Consider registering the path for cleanup as soon as it is created:
```typescript
async function makeLauncherFixture(fixtureRoots: string[]): Promise<string> {
const fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-launcher-"));
fixtureRoots.push(fixtureRoot); // register immediately so afterEach can clean it up
await fs.copyFile(
path.resolve(process.cwd(), "openclaw.mjs"),
path.join(fixtureRoot, "openclaw.mjs"),
);
await fs.mkdir(path.join(fixtureRoot, "dist"), { recursive: true });
return fixtureRoot;
}
```
Each test would then pass `fixtureRoots` to the helper instead of pushing after the fact.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: b53818c |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dc3411350
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26f6757a3c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
* Fix launcher startup regressions * Fix CI follow-up regressions * Fix review follow-ups * Fix workflow audit shell inputs * Handle require resolve gaxios misses
1 participant
Summary
Describe the problem and fix in 2–5 bullets:
openclaw.mjsstartup could fail after build when a transitive import was missing, and the launcher incorrectly reported thatdist/entry.(m)jswas missing.startup-memoryCI lane at the launcher boundary and hid the real root cause from developers.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
node openclaw.mjs --helpnow surfaces real transitive import failures instead of always collapsing them into a generic missing-build-output error.startup-memoryCI failures now print a short local repro path and an LLM-ready prompt.Security Impact (required)
Yes/No) NoYes/No) NoYes/No) NoYes/No) NoYes/No) NoYes, explain risk + mitigation:Repro + Verification
Environment
status --jsonSteps
pnpm buildnode openclaw.mjs --helppnpm test:startup:memoryExpected
startup-memoryfailures emit actionable guidance.Actual
startup-memorypasses on this branch.Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
pnpm vitest run src/infra/gaxios-fetch-compat.test.ts test/openclaw-launcher.e2e.test.ts src/index.test.ts,pnpm build,node openclaw.mjs --help,node openclaw.mjs status --json --timeout 1, andpnpm test:startup:memory.gaxiosresolution falls back to the legacy fetch shim; launcher preserves transitiveERR_MODULE_NOT_FOUND; true missing build output still reports the friendly launcher message.Review Conversations
Compatibility / Migration
Yes/No) YesYes/No) NoYes/No) NoFailure Recovery (if this breaks)
aa8180156c.openclaw.mjs,src/entry.ts,src/index.ts,src/infra/gaxios-fetch-compat.ts,.github/workflows/ci.yml,scripts/check-cli-startup-memory.mjs.openclaw.mjs --helpfailing after build, generic missing-dist errors masking transitive import failures, or startup-memory failures without remediation guidance.Risks and Mitigations
gaxiospath in nested dependency layouts.