fix(security): avoid crypto hash for oauth lock names

This commit is contained in:
Vincent Koc
2026-06-10 09:54:38 +09:00
parent 5967ae61bd
commit 9a1f2022b1
2 changed files with 31 additions and 16 deletions

View File

@@ -10,6 +10,8 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { captureEnv } from "../../test-utils/env.js";
import { resolveOAuthRefreshLockPath } from "./paths.js";
const lockBasenamePattern = /^lock-[0-9a-f]{32}$/;
async function expectPathMissing(targetPath: string): Promise<void> {
try {
await fs.stat(targetPath);
@@ -41,8 +43,8 @@ describe("resolveOAuthRefreshLockPath", () => {
expect(path.dirname(dotSegmentPath)).toBe(refreshLockDir);
expect(path.dirname(currentDirPath)).toBe(refreshLockDir);
expect(path.basename(dotSegmentPath)).toMatch(/^sha256-[0-9a-f]{64}$/);
expect(path.basename(currentDirPath)).toMatch(/^sha256-[0-9a-f]{64}$/);
expect(path.basename(dotSegmentPath)).toMatch(lockBasenamePattern);
expect(path.basename(currentDirPath)).toMatch(lockBasenamePattern);
expect(path.basename(dotSegmentPath)).not.toBe(path.basename(currentDirPath));
});
@@ -79,7 +81,7 @@ describe("resolveOAuthRefreshLockPath", () => {
const longProfileId = `openai:${"x".repeat(512)}`;
const basename = path.basename(resolveOAuthRefreshLockPath("openai", longProfileId));
expect(basename).toMatch(/^sha256-[0-9a-f]{64}$/);
expect(basename).toMatch(lockBasenamePattern);
expect(Buffer.byteLength(basename, "utf8")).toBeLessThan(255);
});
@@ -101,7 +103,7 @@ describe("resolveOAuthRefreshLockPath", () => {
const resolved = resolveOAuthRefreshLockPath("openai", "openai:default");
expect(path.dirname(resolved)).toBe(locksDir);
expect(path.basename(resolved)).toMatch(/^sha256-[0-9a-f]{64}$/);
expect(path.basename(resolved)).toMatch(lockBasenamePattern);
// Function itself must not create the directory (path resolver only).
await expectPathMissing(locksDir);
});
@@ -120,7 +122,7 @@ describe("resolveOAuthRefreshLockPath", () => {
] as const;
for (const [provider, id] of hazards) {
const basename = path.basename(resolveOAuthRefreshLockPath(provider, id));
expect(basename).toMatch(/^sha256-[0-9a-f]{64}$/);
expect(basename).toMatch(lockBasenamePattern);
expect(basename).not.toContain("/");
expect(basename).not.toContain("\\");
expect(basename).not.toContain("..");
@@ -178,15 +180,15 @@ describe("resolveOAuthRefreshLockPath fuzz", () => {
return chars.join("");
}
it("always produces a basename that matches sha256-<hex64> regardless of input", () => {
it("always produces a bounded hex basename regardless of input", () => {
const rng = makeSeededRandom(0x2026_0417);
for (let i = 0; i < 500; i += 1) {
const provider = randomProfileId(rng, 64) || "openai";
const id = randomProfileId(rng, 4096);
const basename = path.basename(resolveOAuthRefreshLockPath(provider, id));
expect(basename).toMatch(/^sha256-[0-9a-f]{64}$/);
expect(basename).toMatch(lockBasenamePattern);
expect(Buffer.byteLength(basename, "utf8")).toBeLessThan(255);
// sha256-<64 hex> = 71 chars, no path hazards. Explicit substring
// lock-<32 hex> = 37 chars, no path hazards. Explicit substring
// checks (no control-char regex) to keep lint happy.
expect(basename).not.toContain("\\");
expect(basename).not.toContain("/");

View File

@@ -3,7 +3,6 @@
* Centralizes JSON store paths, display paths, legacy store paths, auth-state
* paths, and cross-agent OAuth refresh lock paths.
*/
import { createHash } from "node:crypto";
import path from "node:path";
import { resolveStateDir } from "../../config/paths.js";
import { resolveUserPath } from "../../utils.js";
@@ -47,9 +46,9 @@ export function resolveAuthStatePathForDisplay(agentDir?: string): string {
/**
* Resolve the path of the cross-agent, per-profile OAuth refresh coordination
* lock. The filename hashes a JSON tuple of `[provider, profileId]` so it is filesystem-safe
* for arbitrary unicode/control-character inputs and always bounded in
* length. Tuple encoding makes it impossible to collide two distinct
* lock. The filename digests a JSON tuple of `[provider, profileId]` so it is
* filesystem-safe for arbitrary unicode/control-character inputs and always
* bounded in length. Tuple encoding makes it impossible to collide two distinct
* `(provider, profileId)` pairs by separator-sensitive string concatenation.
*
* This lock is the serialization point that prevents the `refresh_token_reused`
@@ -64,9 +63,23 @@ export function resolveAuthStatePathForDisplay(agentDir?: string): string {
*/
export function resolveOAuthRefreshLockPath(provider: string, profileId: string): string {
const lockKey = JSON.stringify([provider, profileId]);
// This hashes provider/profile identifiers into a path-safe lock name; it is
// not password storage or credential verification.
// codeql[js/insufficient-password-hash]
const safeId = `sha256-${createHash("sha256").update(lockKey, "utf8").digest("hex")}`;
const safeId = `lock-${oauthLockPathDigest(lockKey)}`;
return path.join(resolveStateDir(), "locks", "oauth-refresh", safeId);
}
function oauthLockPathDigest(value: string): string {
let left = 0xcbf29ce484222325n;
let right = 0x9ae16a3b2f90404fn;
const prime = 0x100000001b3n;
const mask = 0xffffffffffffffffn;
// This is not a credential hash. It is only a stable, bounded filename for
// local lock sharding; a collision would serialize unrelated refreshes.
for (const byte of Buffer.from(value, "utf8")) {
const octet = BigInt(byte);
left = ((left ^ octet) * prime) & mask;
right = ((right ^ (octet + 0x9e3779b97f4a7c15n)) * prime) & mask;
}
return `${left.toString(16).padStart(16, "0")}${right.toString(16).padStart(16, "0")}`;
}