diff --git a/.changeset/core-session-prefix-sanitize.md b/.changeset/core-session-prefix-sanitize.md new file mode 100644 index 0000000000..93d26baa9b --- /dev/null +++ b/.changeset/core-session-prefix-sanitize.md @@ -0,0 +1,5 @@ +--- +"@aoagents/ao-core": patch +--- + +Harden session prefix generation by sanitizing inputs inside `generateSessionPrefix` via `sanitizeIdentifierComponent`, and derive prefixes from full paths via `deriveSessionPrefixFromProjectPath` when basenames collapse to the generic `project` fallback — covering global registry registration and local wrapped YAML. Legacy wrapped `storageKey` values append a short path fingerprint in that fallback case so distinct risky paths do not share the same storage directory key. diff --git a/packages/core/__tests__/config.test.ts b/packages/core/__tests__/config.test.ts index 041aac7ef1..de5e62ac15 100644 --- a/packages/core/__tests__/config.test.ts +++ b/packages/core/__tests__/config.test.ts @@ -2,7 +2,12 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { mkdirSync, writeFileSync, rmSync, realpathSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; -import { loadConfig, findConfigFile, validateConfig } from "../src/config.js"; +import { + loadConfig, + findConfigFile, + validateConfig, + generateLegacyWrappedStorageKey, +} from "../src/config.js"; import { ConfigNotFoundError } from "../src/types.js"; describe("Config Loading", () => { @@ -233,6 +238,74 @@ projects: expect(Object.values(config.projects)).toEqual([]); expect(Object.values(config.degradedProjects)).toHaveLength(1); }); + + it("sanitizes wrapped dot-path handling for storage key and derived sessionPrefix", () => { + const configPath = join(testDir, "dot-path-config.yaml"); + writeFileSync( + configPath, + [ + "projects:", + " dot-project:", + " path: .", + "", + ].join("\n"), + ); + + const config = loadConfig(configPath); + const project = config.projects["dot-project"]; + const storageKey = generateLegacyWrappedStorageKey(configPath, "."); + + expect(project).toBeDefined(); + if (!project) { + throw new Error("dot-project missing from loaded config"); + } + expect(storageKey).toMatch(/^[a-z0-9]{12}-[a-zA-Z0-9_-]+$/); + expect(storageKey).not.toContain("."); + expect(project.sessionPrefix).toMatch(/^[a-zA-Z0-9_-]+$/); + expect(project.sessionPrefix).not.toBe("."); + }); + + it("disambiguates legacy wrapped storage keys when the sanitized basename is the generic fallback", () => { + const configPath = join(testDir, "legacy-fingerprint.yaml"); + writeFileSync(configPath, "projects: {}\n"); + mkdirSync(join(testDir, "nested"), { recursive: true }); + + const fsRootKey = generateLegacyWrappedStorageKey(configPath, "/"); + expect(fsRootKey).toMatch(/^[a-f0-9]{12}-project-[a-f0-9]{8}$/); + + const normalKey = generateLegacyWrappedStorageKey(configPath, "nested"); + expect(normalKey).toMatch(/^[a-f0-9]{12}-nested$/); + expect(normalKey).not.toBe(fsRootKey); + }); + + it("does not collapse distinct risky paths to the same legacy wrapped storageKey", () => { + const configPath = join(testDir, "risky-paths.yaml"); + writeFileSync( + configPath, + [ + "projects:", + " fs-root:", + " path: /", + " parent-relative:", + " path: ..", + "", + ].join("\n"), + ); + + const config = loadConfig(configPath); + const a = config.projects["fs-root"]; + const b = config.projects["parent-relative"]; + expect(a?.sessionPrefix).toBeDefined(); + expect(b?.sessionPrefix).toBeDefined(); + expect(a!.sessionPrefix).not.toBe(b!.sessionPrefix); + + // Zod-validated config strips unknown keys — storageKey is injected pre-parse only. + const storageKeyFs = generateLegacyWrappedStorageKey(configPath, "/"); + const storageKeyParent = generateLegacyWrappedStorageKey(configPath, ".."); + expect(storageKeyFs).not.toBe(storageKeyParent); + expect(storageKeyFs).toMatch(/^[a-z0-9]{12}-[a-zA-Z0-9_-]+$/); + expect(storageKeyParent).toMatch(/^[a-z0-9]{12}-[a-zA-Z0-9_-]+$/); + }); }); describe("Config Discovery Priority", () => { diff --git a/packages/core/src/__tests__/global-config.test.ts b/packages/core/src/__tests__/global-config.test.ts index 2e7c516282..ee568a8d74 100644 --- a/packages/core/src/__tests__/global-config.test.ts +++ b/packages/core/src/__tests__/global-config.test.ts @@ -210,6 +210,29 @@ describe("global-config storage identity", () => { expect(config?.projects[idB]?.sessionPrefix).toBe("ao-1"); }); + it("re-derives invalid stored session prefixes from the registered path", () => { + const repoPath = createRepo("dot-path-project"); + writeFileSync( + configPath, + [ + "projects:", + " dot:", + " projectId: dot", + ` path: ${repoPath}`, + " displayName: Dot", + " sessionPrefix: .", + "notifiers: {}", + "notificationRouting: {}", + "reactions: {}", + "", + ].join("\n"), + ); + + const resolved = resolveProjectIdentity("dot", loadGlobalConfig(configPath)!, configPath); + + expect(resolved?.sessionPrefix).toBe("dpp"); + }); + it("strips stale shadow fields from legacy entries and rewrites the config", () => { const repoPath = createRepo("legacy", "https://github.com/OpenAI/demo.git"); writeFileSync( diff --git a/packages/core/src/__tests__/migration-storage-v2.test.ts b/packages/core/src/__tests__/migration-storage-v2.test.ts index 915431ce96..3e2d1376e6 100644 --- a/packages/core/src/__tests__/migration-storage-v2.test.ts +++ b/packages/core/src/__tests__/migration-storage-v2.test.ts @@ -1371,6 +1371,44 @@ describe.skipIf(process.platform === "win32")("migration edge cases", () => { } }); + it("blocks migration when active sessions use a re-derived prefix for an invalid stored prefix", async () => { + const projectPath = join(testDir, "dot-path-project"); + mkdirSync(projectPath, { recursive: true }); + writeFileSync( + configPath, + [ + "projects:", + " dot:", + ` path: ${projectPath}`, + " sessionPrefix: .", + "", + ].join("\n"), + ); + + vi.resetModules(); + // The invalid stored prefix "." should be ignored; the path-derived prefix + // for "dot-path-project" is "dpp", matching the active session below. + vi.doMock("node:child_process", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, execSync: vi.fn(() => "dpp-1\n") }; + }); + + const { migrateStorage: migrateStorageWithMock } = await import("../migration/storage-v2.js"); + + try { + await expect( + migrateStorageWithMock({ + aoBaseDir, + globalConfigPath: configPath, + log: () => {}, + }), + ).rejects.toThrow(/active AO tmux session/); + } finally { + vi.doUnmock("node:child_process"); + vi.resetModules(); + } + }); + it("migrates worktree directories to new layout", async () => { const hashDir = join(aoBaseDir, "aaaaaa000000-myproject"); mkdirSync(join(hashDir, "sessions"), { recursive: true }); diff --git a/packages/core/src/__tests__/paths.test.ts b/packages/core/src/__tests__/paths.test.ts index 12f5bfe3c7..906746ebc3 100644 --- a/packages/core/src/__tests__/paths.test.ts +++ b/packages/core/src/__tests__/paths.test.ts @@ -1,9 +1,11 @@ import { describe, expect, it } from "vitest"; import { join } from "node:path"; import { + deriveSessionPrefixFromProjectPath, generateProjectId, generateSessionName, generateSessionPrefix, + sanitizeIdentifierComponent, generateTmuxName, getArchiveDir, getFeedbackReportsDir, @@ -36,9 +38,25 @@ describe("paths", () => { it("keeps session prefix generation unchanged", () => { expect(generateSessionPrefix("agent-orchestrator")).toBe("ao"); expect(generateSessionPrefix("Integrator")).toBe("int"); + expect(generateSessionPrefix("MyApp")).toBe("ma"); expect(generateSessionName("ao", 7)).toBe("ao-7"); }); + it("sanitizes path-like fragments before deriving session prefixes", () => { + expect(sanitizeIdentifierComponent(".")).toBe("project"); + expect(sanitizeIdentifierComponent("/")).toBe("project"); + expect(sanitizeIdentifierComponent("..")).toBe("project"); + expect(generateSessionPrefix(".")).toBe("pro"); + expect(generateSessionPrefix("..")).toBe("pro"); + }); + + it("deriveSessionPrefixFromProjectPath avoids collisions for distinct risky paths", () => { + expect(deriveSessionPrefixFromProjectPath("/")).not.toBe( + deriveSessionPrefixFromProjectPath(join(process.cwd(), "..")), + ); + expect(deriveSessionPrefixFromProjectPath("/")).toMatch(/^[a-zA-Z0-9_-]+$/); + }); + it("uses the storage key as the tmux hash segment", () => { const tmuxName = generateTmuxName(storageKey, "ao", 3); expect(tmuxName).toBe("aaaaaaaaaaaa-ao-3"); diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts index 145a7b2cb5..9bac7658d4 100644 --- a/packages/core/src/config.ts +++ b/packages/core/src/config.ts @@ -24,7 +24,11 @@ import { type LoadedConfig, type OrchestratorConfig, } from "./types.js"; -import { generateSessionPrefix } from "./paths.js"; +import { + deriveSessionPrefixFromProjectPath, + generateSessionPrefix, + sanitizeIdentifierComponent, +} from "./paths.js"; import { getDefaultRuntime } from "./platform.js"; import { getGlobalConfigPath, @@ -74,11 +78,19 @@ function classifyConfigShape(configPath: string): "wrapped" | "flat-or-nonobject : "flat-or-nonobject"; } -function generateLegacyWrappedStorageKey(configPath: string, projectPath: string): string { +export function generateLegacyWrappedStorageKey(configPath: string, projectPath: string): string { const resolvedConfigPath = realpathSync(configPath); const configDir = dirname(resolvedConfigPath); const hash = createHash("sha256").update(configDir).digest("hex").slice(0, 12); - return `${hash}-${basename(projectPath)}`; + const resolvedProjectPath = resolve(configDir, projectPath); + const projectBasename = basename(resolvedProjectPath); + let component = sanitizeIdentifierComponent(projectBasename); + // Avoid collisions when distinct paths sanitize to the generic fallback (e.g. "/" vs ".."). + if (component === "project") { + const pathFingerprint = createHash("sha256").update(resolvedProjectPath).digest("hex").slice(0, 8); + component = `project-${pathFingerprint}`; + } + return `${hash}-${component}`; } function applyWrappedLocalStorageKeys(configPath: string, parsed: unknown): unknown { @@ -595,7 +607,7 @@ function applyProjectDefaults(config: OrchestratorConfig): OrchestratorConfig { // This preserves the long-standing semantics on this branch, where // `/repos/integrator` becomes `int` regardless of the config key. if (!project.sessionPrefix) { - project.sessionPrefix = generateSessionPrefix(basename(project.path)); + project.sessionPrefix = deriveSessionPrefixFromProjectPath(project.path); } const inferredPlugin = inferScmPlugin(project); diff --git a/packages/core/src/global-config.ts b/packages/core/src/global-config.ts index f5840cc39c..1ab8a45a05 100644 --- a/packages/core/src/global-config.ts +++ b/packages/core/src/global-config.ts @@ -8,7 +8,11 @@ import { atomicWriteFileSync } from "./atomic-write.js"; import { detectScmPlatform } from "./config-generator.js"; import { withFileLockSync } from "./file-lock.js"; import { ProjectResolveError, type ProjectResolveErrorKind } from "./types.js"; -import { generateSessionPrefix } from "./paths.js"; +import { + deriveSessionPrefixFromProjectPath, + generateSessionPrefix, + sanitizeIdentifierComponent, +} from "./paths.js"; import { normalizeOriginUrl } from "./storage-key.js"; import { getDefaultRuntime } from "./platform.js"; import { recordActivityEvent } from "./activity-events.js"; @@ -695,7 +699,13 @@ function normalizeLegacyRepoValue( } function getRegisteredSessionPrefix(entry: GlobalProjectEntry, projectId: string): string { - return entry.sessionPrefix ?? generateSessionPrefix(basename(entry.path ?? projectId)); + if (entry.sessionPrefix && sanitizeIdentifierComponent(entry.sessionPrefix) === entry.sessionPrefix) { + return entry.sessionPrefix; + } + if (entry.path?.trim()) { + return deriveSessionPrefixFromProjectPath(resolve(entry.path.trim())); + } + return generateSessionPrefix(projectId); } function findSessionPrefixOwner( @@ -810,10 +820,9 @@ export function registerProjectInGlobalConfig( ?? normalizeRepoIdentity(originUrl) ?? (localConfig?.repo ? normalizeLegacyRepoValue(localConfig.repo) : undefined); const defaultBranch = existing?.defaultBranch ?? localConfig?.defaultBranch ?? "main"; - const requestedSessionPrefix = - existing?.sessionPrefix ?? - localConfig?.sessionPrefix ?? - generateSessionPrefix(basename(requestedProjectPath)); + const requestedSessionPrefix = existing + ? getRegisteredSessionPrefix(existing, effectiveProjectId) + : localConfig?.sessionPrefix ?? deriveSessionPrefixFromProjectPath(requestedProjectPath); const source = existing?.source ?? (repoIdentity ? "ao-project-add" : "local"); const registeredAt = existing?.registeredAt ?? Math.floor(Date.now() / 1000); const explicitSessionPrefix = !existing?.sessionPrefix && Boolean(localConfig?.sessionPrefix); @@ -906,10 +915,7 @@ export function resolveProjectIdentity( const projectPath = entry.path; const name = (entry.displayName as string | undefined) ?? projectId; - const sessionPrefix = - typeof entry.sessionPrefix === "string" && entry.sessionPrefix.length > 0 - ? entry.sessionPrefix - : generateSessionPrefix(basename(projectPath)); + const sessionPrefix = getRegisteredSessionPrefix(entry, projectId); const defaultBranch = typeof entry.defaultBranch === "string" && entry.defaultBranch.length > 0 ? entry.defaultBranch diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 4ca21c112c..11a12a79c0 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -351,6 +351,8 @@ export { // Legacy path functions (deprecated — migration only) generateConfigHash, generateProjectId, + sanitizeIdentifierComponent, + deriveSessionPrefixFromProjectPath, generateSessionPrefix, getProjectBaseDir, getSessionsDir, diff --git a/packages/core/src/migration/storage-v2.ts b/packages/core/src/migration/storage-v2.ts index d8df92aac8..a0e37d4879 100644 --- a/packages/core/src/migration/storage-v2.ts +++ b/packages/core/src/migration/storage-v2.ts @@ -23,11 +23,15 @@ import { unlinkSync, type Dirent, } from "node:fs"; -import { basename, join } from "node:path"; +import { basename, join, resolve } from "node:path"; import { homedir } from "node:os"; import { parse as parseYaml, stringify as stringifyYaml } from "yaml"; import { parseKeyValueContent } from "../key-value.js"; -import { generateSessionPrefix } from "../paths.js"; +import { + deriveSessionPrefixFromProjectPath, + generateSessionPrefix, + sanitizeIdentifierComponent, +} from "../paths.js"; import { atomicWriteFileSync } from "../atomic-write.js"; import { withFileLockSync } from "../file-lock.js"; import { recordActivityEvent } from "../activity-events.js"; @@ -209,15 +213,23 @@ function extractProjectPrefixes(globalConfigPath?: string): string[] { const projects = parsed?.["projects"] as Record> | undefined; if (!projects || typeof projects !== "object") return []; - return Array.from(new Set(Object.entries(projects).map(([projectId, entry]) => { - if (entry && typeof entry["sessionPrefix"] === "string" && entry["sessionPrefix"].trim()) { - return entry["sessionPrefix"].trim(); - } - if (entry && typeof entry["path"] === "string" && entry["path"].trim()) { - return generateSessionPrefix(basename(entry["path"].trim())); - } - return generateSessionPrefix(projectId); - }))); + return Array.from( + new Set( + Object.entries(projects).map(([projectId, entry]) => { + const storedPrefix = + entry && typeof entry["sessionPrefix"] === "string" + ? entry["sessionPrefix"].trim() + : ""; + if (storedPrefix && sanitizeIdentifierComponent(storedPrefix) === storedPrefix) { + return storedPrefix; + } + if (entry && typeof entry["path"] === "string" && entry["path"].trim()) { + return deriveSessionPrefixFromProjectPath(resolve(entry["path"].trim())); + } + return generateSessionPrefix(projectId); + }), + ), + ); } catch { return []; } diff --git a/packages/core/src/paths.ts b/packages/core/src/paths.ts index 40fdc6ce73..b766b5934a 100644 --- a/packages/core/src/paths.ts +++ b/packages/core/src/paths.ts @@ -52,9 +52,24 @@ export function generateProjectId(projectPath: string): string { return basename(projectPath); } +/** Safe fragment for session prefixes, tmux names, and legacy storage-key suffix segments. */ +const IDENTIFIER_COMPONENT_PATTERN = /^[a-zA-Z0-9_-]+$/; + +/** + * Normalize an arbitrary basename/path fragment into `[a-zA-Z0-9_-]+`. + * Used by session prefix generation and legacy wrapped storage keys so dot paths + * and filesystem punctuation cannot leak invalid characters into runtime names. + */ +export function sanitizeIdentifierComponent(value: string): string { + const sanitized = value.replace(/[^a-zA-Z0-9_-]+/g, "-").replace(/^-+|-+$/g, ""); + return IDENTIFIER_COMPONENT_PATTERN.test(sanitized) ? sanitized : "project"; +} + /** * Generate session prefix from project ID using clean heuristics. * + * Input is sanitized first so callers may pass raw path basenames safely. + * * Rules: * 1. ≤4 chars: use as-is (lowercase) * 2. CamelCase: extract uppercase letters (PyTorch → pt) @@ -62,20 +77,22 @@ export function generateProjectId(projectPath: string): string { * 4. Single word: first 3 chars (integrator → int) */ export function generateSessionPrefix(projectId: string): string { - if (projectId.length <= 4) { - return projectId.toLowerCase(); + const safe = sanitizeIdentifierComponent(projectId.trim()); + + if (safe.length <= 4) { + return safe.toLowerCase(); } // CamelCase: extract uppercase letters - const uppercase = projectId.match(/[A-Z]/g); + const uppercase = safe.match(/[A-Z]/g); if (uppercase && uppercase.length > 1) { return uppercase.join("").toLowerCase(); } // kebab-case or snake_case: use initials - if (projectId.includes("-") || projectId.includes("_")) { - const separator = projectId.includes("-") ? "-" : "_"; - return projectId + if (safe.includes("-") || safe.includes("_")) { + const separator = safe.includes("-") ? "-" : "_"; + return safe .split(separator) .map((word) => word[0]) .join("") @@ -83,7 +100,24 @@ export function generateSessionPrefix(projectId: string): string { } // Single word: first 3 characters - return projectId.slice(0, 3).toLowerCase(); + return safe.slice(0, 3).toLowerCase(); +} + +/** + * Derive a tmux-safe session prefix from an on-disk project path. + * + * When the path basename sanitizes to the generic `"project"` fallback (e.g. `"."`, + * `".."`, `"/"`), prefixes would otherwise collide (`generateSessionPrefix("project")` → `pro`). + * In that case we seed prefix generation from a stable synthetic token keyed by the resolved path. + */ +export function deriveSessionPrefixFromProjectPath(absoluteOrRelativeProjectPath: string): string { + const resolved = resolve(absoluteOrRelativeProjectPath); + const base = basename(resolved); + if (sanitizeIdentifierComponent(base) !== "project") { + return generateSessionPrefix(base); + } + const suffix = createHash("sha256").update(resolved).digest("hex").slice(0, 8); + return generateSessionPrefix(`x-${suffix}`); } // =============================================================================