mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 10:10:45 +00:00
fix(auth): close codex review gaps
This commit is contained in:
committed by
Peter Steinberger
parent
859eb06662
commit
78288e37ed
@@ -1,2 +1,2 @@
|
||||
445130135f0037ca2f0877428d58deedf7a7f50e588af5505c1ba09d346663ae plugin-sdk-api-baseline.json
|
||||
147f6f63b835a92e24d6c93b91b0e2adbe1b8fb381d3bd45ef1ae63fd9b3386e plugin-sdk-api-baseline.jsonl
|
||||
57d6ea3acdbaff64d59293bb42224be02ca975c5554afacebb78677642355b6f plugin-sdk-api-baseline.json
|
||||
faff507780c473574e22e73af63dcdc4f4043cea72fa3475606cd4eaf20c1b17 plugin-sdk-api-baseline.jsonl
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import crypto from "node:crypto";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
@@ -15,6 +16,13 @@ let bridgeCodexAppServerStartOptions: typeof import("./auth-bridge.js").bridgeCo
|
||||
|
||||
describe("bridgeCodexAppServerStartOptions", () => {
|
||||
const tempDirs: string[] = [];
|
||||
const resolveHashedCodexHome = (agentDir: string, profileId: string) =>
|
||||
path.join(
|
||||
agentDir,
|
||||
"harness-auth",
|
||||
"codex",
|
||||
crypto.createHash("sha256").update(profileId).digest("hex").slice(0, 16),
|
||||
);
|
||||
|
||||
beforeAll(async () => {
|
||||
({ bridgeCodexAppServerStartOptions } = await import("./auth-bridge.js"));
|
||||
@@ -74,6 +82,10 @@ describe("bridgeCodexAppServerStartOptions", () => {
|
||||
account_id: "acct-123",
|
||||
},
|
||||
});
|
||||
if (process.platform !== "win32") {
|
||||
const authStat = await fs.stat(path.join(result.env?.CODEX_HOME ?? "", "auth.json"));
|
||||
expect(authStat.mode & 0o777).toBe(0o600);
|
||||
}
|
||||
});
|
||||
|
||||
it("leaves start options unchanged when canonical oauth is unavailable", async () => {
|
||||
@@ -97,4 +109,35 @@ describe("bridgeCodexAppServerStartOptions", () => {
|
||||
}),
|
||||
).resolves.toEqual(startOptions);
|
||||
});
|
||||
|
||||
it("refuses to overwrite a symlinked auth bridge file", async () => {
|
||||
const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-codex-app-server-"));
|
||||
tempDirs.push(agentDir);
|
||||
mocks.ensureAuthProfileStore.mockReturnValue({
|
||||
version: 1,
|
||||
profiles: {
|
||||
"openai-codex:default": {
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "access-token",
|
||||
refresh: "refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const codexHome = resolveHashedCodexHome(agentDir, "openai-codex:default");
|
||||
await fs.mkdir(codexHome, { recursive: true });
|
||||
await fs.symlink(path.join(agentDir, "outside.txt"), path.join(codexHome, "auth.json"));
|
||||
|
||||
await expect(
|
||||
bridgeCodexAppServerStartOptions({
|
||||
startOptions: {
|
||||
command: "codex",
|
||||
args: ["app-server"],
|
||||
},
|
||||
agentDir,
|
||||
}),
|
||||
).rejects.toThrow("must not be a symlink");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import crypto from "node:crypto";
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { ensureAuthProfileStore, type OAuthCredential } from "openclaw/plugin-sdk/provider-auth";
|
||||
import { writePrivateSecretFileAtomic } from "openclaw/plugin-sdk/secret-file-runtime";
|
||||
import type { CodexAppServerStartOptions } from "./config.js";
|
||||
|
||||
const DEFAULT_CODEX_AUTH_PROFILE_ID = "openai-codex:default";
|
||||
@@ -60,8 +60,11 @@ export async function bridgeCodexAppServerStartOptions(params: {
|
||||
}
|
||||
|
||||
const codexHome = resolveCodexBridgeHome(params.agentDir, profileId);
|
||||
await fs.mkdir(codexHome, { recursive: true });
|
||||
await fs.writeFile(path.join(codexHome, "auth.json"), buildCodexAuthFile(credential));
|
||||
await writePrivateSecretFileAtomic({
|
||||
rootDir: params.agentDir,
|
||||
filePath: path.join(codexHome, "auth.json"),
|
||||
content: buildCodexAuthFile(credential),
|
||||
});
|
||||
|
||||
return {
|
||||
...params.startOptions,
|
||||
|
||||
@@ -137,6 +137,33 @@ describe("maybeCompactCodexAppServerSession", () => {
|
||||
|
||||
expect(seenAuthProfileId).toBe("openai-codex:work");
|
||||
});
|
||||
|
||||
it("fails closed when the persisted binding auth profile disagrees with the runtime request", async () => {
|
||||
const fake = createFakeCodexClient();
|
||||
const factory = vi.fn(async () => fake.client);
|
||||
__testing.setCodexAppServerClientFactoryForTests(factory);
|
||||
const sessionFile = path.join(tempDir, "session.jsonl");
|
||||
await writeCodexAppServerBinding(sessionFile, {
|
||||
threadId: "thread-1",
|
||||
cwd: tempDir,
|
||||
authProfileId: "openai-codex:binding",
|
||||
});
|
||||
|
||||
const result = await maybeCompactCodexAppServerSession({
|
||||
sessionId: "session-1",
|
||||
sessionKey: "agent:main:session-1",
|
||||
sessionFile,
|
||||
workspaceDir: tempDir,
|
||||
authProfileId: "openai-codex:runtime",
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
ok: false,
|
||||
compacted: false,
|
||||
reason: "auth profile mismatch for session binding",
|
||||
});
|
||||
expect(factory).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
function createFakeCodexClient(): {
|
||||
|
||||
@@ -38,8 +38,19 @@ export async function maybeCompactCodexAppServerSession(
|
||||
if (!binding?.threadId) {
|
||||
return { ok: false, compacted: false, reason: "no codex app-server thread binding" };
|
||||
}
|
||||
const requestedAuthProfileId = params.authProfileId?.trim() || undefined;
|
||||
if (
|
||||
requestedAuthProfileId &&
|
||||
binding.authProfileId &&
|
||||
binding.authProfileId !== requestedAuthProfileId
|
||||
) {
|
||||
return { ok: false, compacted: false, reason: "auth profile mismatch for session binding" };
|
||||
}
|
||||
|
||||
const client = await clientFactory(appServer.start, binding.authProfileId);
|
||||
const client = await clientFactory(
|
||||
appServer.start,
|
||||
requestedAuthProfileId ?? binding.authProfileId,
|
||||
);
|
||||
const waiter = createCodexNativeCompactionWaiter(client, binding.threadId);
|
||||
let completion: CodexNativeCompactionCompletion;
|
||||
try {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import crypto from "node:crypto";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
@@ -6,6 +7,13 @@ import { prepareOpenAICodexCliExecution } from "./openai-codex-cli-bridge.js";
|
||||
|
||||
describe("prepareOpenAICodexCliExecution", () => {
|
||||
const tempDirs: string[] = [];
|
||||
const resolveHashedCodexHome = (agentDir: string, profileId: string) =>
|
||||
path.join(
|
||||
agentDir,
|
||||
"cli-auth",
|
||||
"codex",
|
||||
crypto.createHash("sha256").update(profileId).digest("hex").slice(0, 16),
|
||||
);
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(
|
||||
@@ -52,6 +60,10 @@ describe("prepareOpenAICodexCliExecution", () => {
|
||||
account_id: "acct-123",
|
||||
},
|
||||
});
|
||||
if (process.platform !== "win32") {
|
||||
const authStat = await fs.stat(path.join(result?.env?.CODEX_HOME ?? "", "auth.json"));
|
||||
expect(authStat.mode & 0o777).toBe(0o600);
|
||||
}
|
||||
});
|
||||
|
||||
it("returns null when there is no bridgeable canonical oauth credential", async () => {
|
||||
@@ -74,4 +86,30 @@ describe("prepareOpenAICodexCliExecution", () => {
|
||||
}),
|
||||
).resolves.toBeNull();
|
||||
});
|
||||
|
||||
it("refuses to overwrite a symlinked codex cli auth bridge file", async () => {
|
||||
const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-codex-cli-bridge-"));
|
||||
tempDirs.push(agentDir);
|
||||
const codexHome = resolveHashedCodexHome(agentDir, "openai-codex:default");
|
||||
await fs.mkdir(codexHome, { recursive: true });
|
||||
await fs.symlink(path.join(agentDir, "outside.txt"), path.join(codexHome, "auth.json"));
|
||||
|
||||
await expect(
|
||||
prepareOpenAICodexCliExecution({
|
||||
config: undefined,
|
||||
workspaceDir: agentDir,
|
||||
agentDir,
|
||||
provider: "codex-cli",
|
||||
modelId: "gpt-5.4",
|
||||
authProfileId: "openai-codex:default",
|
||||
authCredential: {
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "access-token",
|
||||
refresh: "refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
},
|
||||
}),
|
||||
).rejects.toThrow("must not be a symlink");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
import crypto from "node:crypto";
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import type {
|
||||
CliBackendPreparedExecution,
|
||||
CliBackendPrepareExecutionContext,
|
||||
} from "openclaw/plugin-sdk/cli-backend";
|
||||
import type { OAuthCredential } from "openclaw/plugin-sdk/provider-auth";
|
||||
import { writePrivateSecretFileAtomic } from "openclaw/plugin-sdk/secret-file-runtime";
|
||||
|
||||
const OPENAI_CODEX_PROVIDER_ID = "openai-codex";
|
||||
const CODEX_AUTH_ENV_CLEAR_KEYS = ["OPENAI_API_KEY"] as const;
|
||||
@@ -60,8 +60,11 @@ export async function prepareOpenAICodexCliExecution(
|
||||
}
|
||||
|
||||
const codexHome = resolveCodexBridgeHome(ctx.agentDir, ctx.authProfileId);
|
||||
await fs.mkdir(codexHome, { recursive: true });
|
||||
await fs.writeFile(path.join(codexHome, "auth.json"), buildCodexAuthFile(ctx.authCredential));
|
||||
await writePrivateSecretFileAtomic({
|
||||
rootDir: ctx.agentDir,
|
||||
filePath: path.join(codexHome, "auth.json"),
|
||||
content: buildCodexAuthFile(ctx.authCredential),
|
||||
});
|
||||
|
||||
return {
|
||||
env: {
|
||||
|
||||
@@ -4,6 +4,7 @@ import { log } from "./constants.js";
|
||||
import {
|
||||
areOAuthCredentialsEquivalent,
|
||||
hasUsableOAuthCredential,
|
||||
isSafeToAdoptBootstrapOAuthIdentity,
|
||||
isSafeToOverwriteStoredOAuthIdentity,
|
||||
shouldBootstrapFromExternalCliCredential,
|
||||
shouldReplaceStoredOAuthCredential,
|
||||
@@ -13,6 +14,7 @@ import type { AuthProfileStore, OAuthCredential } from "./types.js";
|
||||
export {
|
||||
areOAuthCredentialsEquivalent,
|
||||
hasUsableOAuthCredential,
|
||||
isSafeToAdoptBootstrapOAuthIdentity,
|
||||
isSafeToOverwriteStoredOAuthIdentity,
|
||||
shouldBootstrapFromExternalCliCredential,
|
||||
shouldReplaceStoredOAuthCredential,
|
||||
@@ -139,7 +141,7 @@ export function resolveExternalCliAuthProfiles(
|
||||
}
|
||||
if (
|
||||
existingOAuth &&
|
||||
!isSafeToOverwriteStoredOAuthIdentity(existingOAuth, creds) &&
|
||||
!isSafeToAdoptBootstrapOAuthIdentity(existingOAuth, creds) &&
|
||||
!areOAuthCredentialsEquivalent(existingOAuth, creds)
|
||||
) {
|
||||
log.warn("refused external cli oauth bootstrap: identity mismatch or missing binding", {
|
||||
|
||||
@@ -1,5 +1,14 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { isSafeToOverwriteStoredOAuthIdentity, OAuthManagerRefreshError } from "./oauth-manager.js";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
createOAuthManager,
|
||||
isSafeToAdoptBootstrapOAuthIdentity,
|
||||
isSafeToOverwriteStoredOAuthIdentity,
|
||||
OAuthManagerRefreshError,
|
||||
} from "./oauth-manager.js";
|
||||
import { ensureAuthProfileStore, saveAuthProfileStore } from "./store.js";
|
||||
import type { AuthProfileStore, OAuthCredential } from "./types.js";
|
||||
|
||||
function createCredential(overrides: Partial<OAuthCredential> = {}): OAuthCredential {
|
||||
@@ -13,6 +22,12 @@ function createCredential(overrides: Partial<OAuthCredential> = {}): OAuthCreden
|
||||
};
|
||||
}
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(tempDirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true })));
|
||||
});
|
||||
|
||||
describe("isSafeToOverwriteStoredOAuthIdentity", () => {
|
||||
it("accepts matching account identities", () => {
|
||||
expect(
|
||||
@@ -40,6 +55,22 @@ describe("isSafeToOverwriteStoredOAuthIdentity", () => {
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("still allows identity-less external bootstrap adoption", () => {
|
||||
const existing = createCredential({
|
||||
access: "expired-local-access",
|
||||
refresh: "expired-local-refresh",
|
||||
expires: Date.now() - 60_000,
|
||||
});
|
||||
const incoming = createCredential({
|
||||
access: "external-access",
|
||||
refresh: "external-refresh",
|
||||
expires: Date.now() + 60_000,
|
||||
});
|
||||
|
||||
expect(isSafeToOverwriteStoredOAuthIdentity(existing, incoming)).toBe(false);
|
||||
expect(isSafeToAdoptBootstrapOAuthIdentity(existing, incoming)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("OAuthManagerRefreshError", () => {
|
||||
@@ -69,3 +100,63 @@ describe("OAuthManagerRefreshError", () => {
|
||||
expect(serialized).not.toContain("store-refresh");
|
||||
});
|
||||
});
|
||||
|
||||
describe("createOAuthManager", () => {
|
||||
it("refreshes with the adopted external oauth credential", async () => {
|
||||
const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "oauth-manager-refresh-"));
|
||||
tempDirs.push(agentDir);
|
||||
const profileId = "minimax-portal:default";
|
||||
const localCredential = createCredential({
|
||||
provider: "minimax-portal",
|
||||
access: "stale-local-access",
|
||||
refresh: "stale-local-refresh",
|
||||
expires: Date.now() - 60_000,
|
||||
});
|
||||
saveAuthProfileStore(
|
||||
{
|
||||
version: 1,
|
||||
profiles: {
|
||||
[profileId]: localCredential,
|
||||
},
|
||||
},
|
||||
agentDir,
|
||||
);
|
||||
|
||||
const manager = createOAuthManager({
|
||||
buildApiKey: async (_provider, credential) => credential.access,
|
||||
refreshCredential: vi.fn(async (credential) => {
|
||||
expect(credential.refresh).toBe("external-refresh");
|
||||
return {
|
||||
access: "rotated-access",
|
||||
refresh: "rotated-refresh",
|
||||
expires: Date.now() + 60_000,
|
||||
};
|
||||
}),
|
||||
readBootstrapCredential: () =>
|
||||
createCredential({
|
||||
provider: "minimax-portal",
|
||||
access: "expired-external-access",
|
||||
refresh: "external-refresh",
|
||||
expires: Date.now() - 30_000,
|
||||
}),
|
||||
isRefreshTokenReusedError: () => false,
|
||||
isSafeToCopyOAuthIdentity: () => true,
|
||||
});
|
||||
|
||||
const result = await manager.resolveOAuthAccess({
|
||||
store: ensureAuthProfileStore(agentDir),
|
||||
profileId,
|
||||
credential: localCredential,
|
||||
agentDir,
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
apiKey: "rotated-access",
|
||||
credential: expect.objectContaining({
|
||||
provider: "minimax-portal",
|
||||
access: "rotated-access",
|
||||
refresh: "rotated-refresh",
|
||||
}),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -187,6 +187,25 @@ export function isSafeToOverwriteStoredOAuthIdentity(
|
||||
return hasMatchingOAuthIdentity(existing, incoming);
|
||||
}
|
||||
|
||||
export function isSafeToAdoptBootstrapOAuthIdentity(
|
||||
existing: OAuthCredential | undefined,
|
||||
incoming: OAuthCredential,
|
||||
): boolean {
|
||||
if (!existing || existing.type !== "oauth") {
|
||||
return true;
|
||||
}
|
||||
if (existing.provider !== incoming.provider) {
|
||||
return false;
|
||||
}
|
||||
if (areOAuthCredentialsEquivalent(existing, incoming)) {
|
||||
return true;
|
||||
}
|
||||
if (!hasOAuthIdentity(existing)) {
|
||||
return true;
|
||||
}
|
||||
return hasMatchingOAuthIdentity(existing, incoming);
|
||||
}
|
||||
|
||||
export function shouldBootstrapFromExternalCliCredential(params: {
|
||||
existing: OAuthCredential | undefined;
|
||||
imported: OAuthCredential;
|
||||
@@ -289,7 +308,7 @@ export function resolveEffectiveOAuthCredential(params: {
|
||||
});
|
||||
return params.credential;
|
||||
}
|
||||
if (!isSafeToOverwriteStoredOAuthIdentity(params.credential, imported)) {
|
||||
if (!isSafeToAdoptBootstrapOAuthIdentity(params.credential, imported)) {
|
||||
log.warn("refused external oauth bootstrap credential: identity mismatch or missing binding", {
|
||||
profileId: params.profileId,
|
||||
provider: params.credential.provider,
|
||||
@@ -443,6 +462,7 @@ export function createOAuthManager(adapter: OAuthManagerAdapter) {
|
||||
if (!cred || cred.type !== "oauth") {
|
||||
return null;
|
||||
}
|
||||
let credentialToRefresh = cred;
|
||||
|
||||
if (hasUsableOAuthCredential(cred)) {
|
||||
return {
|
||||
@@ -501,7 +521,7 @@ export function createOAuthManager(adapter: OAuthManagerAdapter) {
|
||||
profileId: params.profileId,
|
||||
provider: cred.provider,
|
||||
});
|
||||
} else if (!isSafeToOverwriteStoredOAuthIdentity(cred, externallyManaged)) {
|
||||
} else if (!isSafeToAdoptBootstrapOAuthIdentity(cred, externallyManaged)) {
|
||||
log.warn(
|
||||
"refused external oauth bootstrap credential: identity mismatch or missing binding",
|
||||
{
|
||||
@@ -517,6 +537,7 @@ export function createOAuthManager(adapter: OAuthManagerAdapter) {
|
||||
store.profiles[params.profileId] = { ...externallyManaged };
|
||||
saveAuthProfileStore(store, params.agentDir);
|
||||
}
|
||||
credentialToRefresh = externallyManaged;
|
||||
if (hasUsableOAuthCredential(externallyManaged)) {
|
||||
return {
|
||||
apiKey: await adapter.buildApiKey(externallyManaged.provider, externallyManaged),
|
||||
@@ -530,10 +551,10 @@ export function createOAuthManager(adapter: OAuthManagerAdapter) {
|
||||
`refreshOAuthCredential(${cred.provider})`,
|
||||
OAUTH_REFRESH_CALL_TIMEOUT_MS,
|
||||
async () => {
|
||||
const refreshed = await adapter.refreshCredential(cred);
|
||||
const refreshed = await adapter.refreshCredential(credentialToRefresh);
|
||||
return refreshed
|
||||
? ({
|
||||
...cred,
|
||||
...credentialToRefresh,
|
||||
...refreshed,
|
||||
type: "oauth",
|
||||
} satisfies OAuthCredential)
|
||||
|
||||
36
src/agents/cli-runner/prepare.test.ts
Normal file
36
src/agents/cli-runner/prepare.test.ts
Normal file
@@ -0,0 +1,36 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { shouldSkipLocalCliCredentialEpoch } from "./prepare.js";
|
||||
|
||||
describe("shouldSkipLocalCliCredentialEpoch", () => {
|
||||
it("skips local cli auth only when a profile-owned execution was prepared", () => {
|
||||
expect(
|
||||
shouldSkipLocalCliCredentialEpoch({
|
||||
authEpochMode: "profile-only",
|
||||
authProfileId: "openai-codex:default",
|
||||
authCredential: {
|
||||
type: "oauth",
|
||||
provider: "openai-codex",
|
||||
access: "access-token",
|
||||
refresh: "refresh-token",
|
||||
expires: Date.now() + 60_000,
|
||||
},
|
||||
preparedExecution: {
|
||||
env: {
|
||||
CODEX_HOME: "/tmp/codex-home",
|
||||
},
|
||||
},
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps local cli auth in the epoch when the selected profile has no bridgeable execution", () => {
|
||||
expect(
|
||||
shouldSkipLocalCliCredentialEpoch({
|
||||
authEpochMode: "profile-only",
|
||||
authProfileId: "openai-codex:default",
|
||||
authCredential: undefined,
|
||||
preparedExecution: null,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -3,6 +3,10 @@ import {
|
||||
createMcpLoopbackServerConfig,
|
||||
getActiveMcpLoopbackRuntime,
|
||||
} from "../../gateway/mcp-http.loopback-runtime.js";
|
||||
import type {
|
||||
CliBackendAuthEpochMode,
|
||||
CliBackendPreparedExecution,
|
||||
} from "../../plugins/cli-backend.types.js";
|
||||
import { resolveOpenClawAgentDir } from "../agent-paths.js";
|
||||
import { resolveSessionAgentIds } from "../agent-scope.js";
|
||||
import { loadAuthProfileStoreForRuntime } from "../auth-profiles/store.js";
|
||||
@@ -51,6 +55,20 @@ export function setCliRunnerPrepareTestDeps(overrides: Partial<typeof prepareDep
|
||||
Object.assign(prepareDeps, overrides);
|
||||
}
|
||||
|
||||
export function shouldSkipLocalCliCredentialEpoch(params: {
|
||||
authEpochMode?: CliBackendAuthEpochMode;
|
||||
authProfileId?: string;
|
||||
authCredential?: AuthProfileCredential;
|
||||
preparedExecution?: CliBackendPreparedExecution | null;
|
||||
}): boolean {
|
||||
return Boolean(
|
||||
params.authEpochMode === "profile-only" &&
|
||||
params.authProfileId &&
|
||||
params.authCredential &&
|
||||
params.preparedExecution,
|
||||
);
|
||||
}
|
||||
|
||||
export async function prepareCliRunContext(
|
||||
params: RunCliAgentParams,
|
||||
): Promise<PreparedCliRunContext> {
|
||||
@@ -88,12 +106,6 @@ export async function prepareCliRunContext(
|
||||
});
|
||||
authCredential = authStore.profiles[effectiveAuthProfileId];
|
||||
}
|
||||
const authEpoch = await resolveCliAuthEpoch({
|
||||
provider: params.provider,
|
||||
authProfileId: effectiveAuthProfileId,
|
||||
skipLocalCredential:
|
||||
backendResolved.authEpochMode === "profile-only" && Boolean(effectiveAuthProfileId),
|
||||
});
|
||||
const extraSystemPrompt = params.extraSystemPrompt?.trim() ?? "";
|
||||
const extraSystemPromptHash = hashCliSessionText(extraSystemPrompt);
|
||||
const modelId = (params.model ?? "default").trim() || "default";
|
||||
@@ -175,6 +187,16 @@ export async function prepareCliRunContext(
|
||||
authProfileId: effectiveAuthProfileId,
|
||||
authCredential,
|
||||
});
|
||||
const authEpoch = await resolveCliAuthEpoch({
|
||||
provider: params.provider,
|
||||
authProfileId: effectiveAuthProfileId,
|
||||
skipLocalCredential: shouldSkipLocalCliCredentialEpoch({
|
||||
authEpochMode: backendResolved.authEpochMode,
|
||||
authProfileId: effectiveAuthProfileId,
|
||||
authCredential,
|
||||
preparedExecution,
|
||||
}),
|
||||
});
|
||||
const preparedBackendEnv =
|
||||
preparedExecution?.env && Object.keys(preparedExecution.env).length > 0
|
||||
? { ...preparedBackend.env, ...preparedExecution.env }
|
||||
|
||||
@@ -1,12 +1,15 @@
|
||||
import { mkdir, symlink, writeFile } from "node:fs/promises";
|
||||
import { mkdir, stat, symlink, writeFile } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js";
|
||||
import {
|
||||
DEFAULT_SECRET_FILE_MAX_BYTES,
|
||||
loadSecretFileSync,
|
||||
PRIVATE_SECRET_DIR_MODE,
|
||||
PRIVATE_SECRET_FILE_MODE,
|
||||
readSecretFileSync,
|
||||
tryReadSecretFileSync,
|
||||
writePrivateSecretFileAtomic,
|
||||
} from "./secret-file.js";
|
||||
|
||||
const tempDirs = createTrackedTempDirs();
|
||||
@@ -187,3 +190,60 @@ describe("readSecretFileSync", () => {
|
||||
expect(tryReadSecretFileSync(file, label, options)).toBe((expected as () => undefined)());
|
||||
});
|
||||
});
|
||||
|
||||
describe("writePrivateSecretFileAtomic", () => {
|
||||
it("writes a private file with owner-only permissions", async () => {
|
||||
const dir = await createTempDir();
|
||||
const file = path.join(dir, "nested", "auth.json");
|
||||
|
||||
await writePrivateSecretFileAtomic({
|
||||
rootDir: dir,
|
||||
filePath: file,
|
||||
content: '{"ok":true}\n',
|
||||
});
|
||||
|
||||
expect(loadSecretFileSync(file, "Gateway password")).toMatchObject({
|
||||
ok: true,
|
||||
secret: '{"ok":true}',
|
||||
});
|
||||
if (process.platform !== "win32") {
|
||||
const dirStat = await stat(path.dirname(file));
|
||||
const fileStat = await stat(file);
|
||||
expect(dirStat.mode & 0o777).toBe(PRIVATE_SECRET_DIR_MODE);
|
||||
expect(fileStat.mode & 0o777).toBe(PRIVATE_SECRET_FILE_MODE);
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects symlinked target files", async () => {
|
||||
const dir = await createTempDir();
|
||||
const nestedDir = path.join(dir, "nested");
|
||||
const target = path.join(dir, "outside.txt");
|
||||
const link = path.join(nestedDir, "auth.json");
|
||||
await mkdir(nestedDir);
|
||||
await writeFile(target, "outside", "utf8");
|
||||
await symlink(target, link);
|
||||
|
||||
await expect(
|
||||
writePrivateSecretFileAtomic({
|
||||
rootDir: dir,
|
||||
filePath: link,
|
||||
content: '{"ok":true}\n',
|
||||
}),
|
||||
).rejects.toThrow("must not be a symlink");
|
||||
});
|
||||
|
||||
it("rejects symlinked path components", async () => {
|
||||
const dir = await createTempDir();
|
||||
const targetDir = path.join(dir, "outside-dir");
|
||||
await mkdir(targetDir);
|
||||
await symlink(targetDir, path.join(dir, "linked"));
|
||||
|
||||
await expect(
|
||||
writePrivateSecretFileAtomic({
|
||||
rootDir: dir,
|
||||
filePath: path.join(dir, "linked", "auth.json"),
|
||||
content: '{"ok":true}\n',
|
||||
}),
|
||||
).rejects.toThrow("must not be a symlink");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,8 +1,13 @@
|
||||
import { randomBytes } from "node:crypto";
|
||||
import fs from "node:fs";
|
||||
import fsp from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { resolveUserPath } from "../utils.js";
|
||||
import { openVerifiedFileSync } from "./safe-open-sync.js";
|
||||
|
||||
export const DEFAULT_SECRET_FILE_MAX_BYTES = 16 * 1024;
|
||||
export const PRIVATE_SECRET_DIR_MODE = 0o700;
|
||||
export const PRIVATE_SECRET_FILE_MODE = 0o600;
|
||||
|
||||
export type SecretFileReadOptions = {
|
||||
maxBytes?: number;
|
||||
@@ -138,3 +143,102 @@ export function tryReadSecretFileSync(
|
||||
const result = loadSecretFileSync(filePath, label, options);
|
||||
return result.ok ? result.secret : undefined;
|
||||
}
|
||||
|
||||
function assertPathWithinRoot(rootDir: string, targetPath: string): void {
|
||||
const relative = path.relative(rootDir, targetPath);
|
||||
if (!relative || relative.startsWith("..") || path.isAbsolute(relative)) {
|
||||
throw new Error(`Private secret path must stay under ${rootDir}.`);
|
||||
}
|
||||
}
|
||||
|
||||
async function ensurePrivateDirectory(rootDir: string, targetDir: string): Promise<void> {
|
||||
const resolvedRoot = path.resolve(rootDir);
|
||||
const resolvedTarget = path.resolve(targetDir);
|
||||
if (resolvedTarget === resolvedRoot) {
|
||||
await fsp.mkdir(resolvedRoot, { recursive: true, mode: PRIVATE_SECRET_DIR_MODE });
|
||||
const rootStat = await fsp.lstat(resolvedRoot);
|
||||
if (rootStat.isSymbolicLink()) {
|
||||
throw new Error(`Private secret root ${resolvedRoot} must not be a symlink.`);
|
||||
}
|
||||
if (!rootStat.isDirectory()) {
|
||||
throw new Error(`Private secret root ${resolvedRoot} must be a directory.`);
|
||||
}
|
||||
await fsp.chmod(resolvedRoot, PRIVATE_SECRET_DIR_MODE).catch(() => undefined);
|
||||
return;
|
||||
}
|
||||
|
||||
assertPathWithinRoot(resolvedRoot, resolvedTarget);
|
||||
await ensurePrivateDirectory(resolvedRoot, resolvedRoot);
|
||||
|
||||
let current = resolvedRoot;
|
||||
for (const segment of path
|
||||
.relative(resolvedRoot, resolvedTarget)
|
||||
.split(path.sep)
|
||||
.filter(Boolean)) {
|
||||
current = path.join(current, segment);
|
||||
try {
|
||||
const stat = await fsp.lstat(current);
|
||||
if (stat.isSymbolicLink()) {
|
||||
throw new Error(`Private secret directory component ${current} must not be a symlink.`);
|
||||
}
|
||||
if (!stat.isDirectory()) {
|
||||
throw new Error(`Private secret directory component ${current} must be a directory.`);
|
||||
}
|
||||
} catch (error) {
|
||||
if (!error || typeof error !== "object" || !("code" in error) || error.code !== "ENOENT") {
|
||||
throw error;
|
||||
}
|
||||
await fsp.mkdir(current, { mode: PRIVATE_SECRET_DIR_MODE });
|
||||
}
|
||||
await fsp.chmod(current, PRIVATE_SECRET_DIR_MODE).catch(() => undefined);
|
||||
}
|
||||
}
|
||||
|
||||
export async function writePrivateSecretFileAtomic(params: {
|
||||
rootDir: string;
|
||||
filePath: string;
|
||||
content: string | Uint8Array;
|
||||
}): Promise<void> {
|
||||
const resolvedRoot = path.resolve(params.rootDir);
|
||||
const resolvedFile = path.resolve(params.filePath);
|
||||
assertPathWithinRoot(resolvedRoot, resolvedFile);
|
||||
const parentDir = path.dirname(resolvedFile);
|
||||
await ensurePrivateDirectory(resolvedRoot, parentDir);
|
||||
|
||||
try {
|
||||
const stat = await fsp.lstat(resolvedFile);
|
||||
if (stat.isSymbolicLink()) {
|
||||
throw new Error(`Private secret file ${resolvedFile} must not be a symlink.`);
|
||||
}
|
||||
if (!stat.isFile()) {
|
||||
throw new Error(`Private secret file ${resolvedFile} must be a regular file.`);
|
||||
}
|
||||
} catch (error) {
|
||||
if (!error || typeof error !== "object" || !("code" in error) || error.code !== "ENOENT") {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
const tempPath = path.join(
|
||||
parentDir,
|
||||
`.tmp-${process.pid}-${Date.now()}-${randomBytes(6).toString("hex")}`,
|
||||
);
|
||||
let createdTemp = false;
|
||||
try {
|
||||
const handle = await fsp.open(tempPath, "wx", PRIVATE_SECRET_FILE_MODE);
|
||||
createdTemp = true;
|
||||
try {
|
||||
await handle.writeFile(params.content);
|
||||
} finally {
|
||||
await handle.close();
|
||||
}
|
||||
await fsp.chmod(tempPath, PRIVATE_SECRET_FILE_MODE).catch(() => undefined);
|
||||
await fsp.rename(tempPath, resolvedFile);
|
||||
createdTemp = false;
|
||||
await fsp.chmod(resolvedFile, PRIVATE_SECRET_FILE_MODE).catch(() => undefined);
|
||||
} finally {
|
||||
if (createdTemp) {
|
||||
await fsp.unlink(tempPath).catch(() => undefined);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,7 +1,10 @@
|
||||
export {
|
||||
DEFAULT_SECRET_FILE_MAX_BYTES,
|
||||
PRIVATE_SECRET_DIR_MODE,
|
||||
PRIVATE_SECRET_FILE_MODE,
|
||||
loadSecretFileSync,
|
||||
readSecretFileSync,
|
||||
writePrivateSecretFileAtomic,
|
||||
tryReadSecretFileSync,
|
||||
} from "../infra/secret-file.js";
|
||||
export type { SecretFileReadOptions, SecretFileReadResult } from "../infra/secret-file.js";
|
||||
|
||||
Reference in New Issue
Block a user