fix(cli): address security and correctness review findings

- Add path traversal guard in registry schema (reject absolute paths and ..)
- Add containment check in add.ts before writing files outside project root
- Validate OAC_PACKAGE_ROOT is absolute path before trusting it
- Add depth limit (10) to findPackageRoot directory walk
- Add schema validation to update-check cache reader
- Wrap writeManifest in try/catch in update.ts
- Normalize manifest keys to POSIX paths with traversal rejection
- Fix empty HOME display bug in status.ts

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

packages/cli/src/commands/add.ts

@@ -144,6 +144,12 @@ const performInstall = async (
   const destPath = path.join(projectRoot, destRelativePath);
   const destDir = path.dirname(destRelativePath);
 
+  // Guard: ensure destination stays inside the project root
+  const resolvedDest = path.resolve(projectRoot, destRelativePath);
+  if (!resolvedDest.startsWith(path.resolve(projectRoot) + path.sep)) {
+    throw new Error(`Refusing to install outside project root: ${resolvedDest}`);
+  }
+
   info(`Installing ${component.type}:${component.id} → ${destDir}/`);
 
   if (opts.verbose) {

packages/cli/src/commands/status.ts

@@ -123,7 +123,7 @@ const printStatus = (
   ides: DetectedIde[],
 ): void => {
   const homeDir = process.env['HOME'] ?? process.env['USERPROFILE'] ?? '';
-  const displayPath = projectRoot.startsWith(homeDir)
+  const displayPath = homeDir && projectRoot.startsWith(homeDir)
     ? `~${projectRoot.slice(homeDir.length)}`
     : projectRoot;
 

packages/cli/src/commands/update.ts

@@ -128,8 +128,16 @@ async function runUpdate(projectRoot: string, opts: UpdateOptions): Promise<Inst
 
   // Write updated manifest for all successfully processed files (not dry-run)
   if (!opts.dryRun) {
-    await writeManifest(projectRoot, updatedManifest);
-    verbose('Manifest written.');
+    try {
+      await writeManifest(projectRoot, updatedManifest);
+      verbose('Manifest written.');
+    } catch (err: unknown) {
+      const msg = err instanceof Error ? err.message : String(err);
+      error(`Failed to write manifest: ${msg}`);
+      error('Fix: check write permissions for the .oac/ directory.');
+      process.exitCode = 1;
+      return result;
+    }
     if (result.errors.length > 0) {
       warn('Some files failed — manifest updated for successful files. Re-run to retry failures.');
     }

packages/cli/src/lib/bundled.ts

@@ -1,6 +1,6 @@
 import { existsSync } from "node:fs";
 import { readdir, stat } from "node:fs/promises";
-import { join, relative } from "node:path";
+import { isAbsolute, join, relative } from "node:path";
 
 // --- Types ---
 
@@ -31,6 +31,9 @@ export function getPackageRoot(): string {
   // In dev, set OAC_PACKAGE_ROOT=/path/to/repo to bypass the walk entirely.
   const envOverride = process.env['OAC_PACKAGE_ROOT'];
   if (envOverride) {
+    if (!isAbsolute(envOverride)) {
+      throw new Error(`OAC_PACKAGE_ROOT must be an absolute path, got: ${envOverride}`);
+    }
     return envOverride;
   }
   // import.meta.dir is Bun's native equivalent of __dirname — points to packages/cli/dist/ at runtime
@@ -54,8 +57,16 @@ export function getPackageRoot(): string {
  */
 export function findPackageRoot(dir: string): string {
   let current = dir;
+  let depth = 0;
+  const MAX_DEPTH = 10;
 
   while (true) {
+    if (++depth > MAX_DEPTH) {
+      throw new Error(
+        `getPackageRoot: exceeded ${MAX_DEPTH} directory levels walking up from "${dir}". ` +
+          `Set OAC_PACKAGE_ROOT env var to bypass the walk.`,
+      );
+    }
     const hasOpencode = existsSync(join(current, ".opencode"));
     const hasPackageJson = existsSync(join(current, "package.json"));
 

packages/cli/src/lib/manifest.ts

@@ -80,14 +80,21 @@ export const addFileToManifest = (
   manifest: ManifestFile,
   filePath: string,
   entry: FileEntry,
-): ManifestFile => ({
-  ...manifest,
-  updatedAt: new Date().toISOString(),
-  files: {
-    ...manifest.files,
-    [filePath]: entry,
-  },
-});
+): ManifestFile => {
+  // Normalize to forward-slash POSIX paths and reject traversal
+  const normalized = filePath.split(path.sep).join('/');
+  if (normalized.includes('..')) {
+    throw new Error(`Refusing to add path with traversal segments: ${filePath}`);
+  }
+  return {
+    ...manifest,
+    updatedAt: new Date().toISOString(),
+    files: {
+      ...manifest.files,
+      [normalized]: entry,
+    },
+  };
+};
 
 /**
  * Returns a new manifest with the given file entry removed.

packages/cli/src/lib/registry.ts

@@ -1,5 +1,5 @@
 import { z } from "zod";
-import { join } from "node:path";
+import { isAbsolute, join } from "node:path";
 
 // ── Constants ──────────────────────────────────────────────────────────────────
 
@@ -36,7 +36,10 @@ export const RegistryComponentSchema = z.object({
   id: z.string(),
   name: z.string(),
   type: ComponentTypeSchema,
-  path: z.string(),
+  path: z.string().refine(
+    (p) => !isAbsolute(p) && !p.includes('..'),
+    { message: 'Component path must be relative and must not contain ..' }
+  ),
   description: z.string(),
   tags: z.array(z.string()).default([]),
   dependencies: z.array(z.string()).default([]),

packages/cli/src/lib/update-check.ts

@@ -17,10 +17,13 @@ type UpdateCache = {
 /** Reads the cached update check result. Returns null if cache is missing or stale. */
 async function readCache(): Promise<UpdateCache | null> {
   try {
-    const raw = (await Bun.file(CACHE_FILE).json()) as UpdateCache
-    const age = Date.now() - new Date(raw.checkedAt).getTime()
+    const raw = await Bun.file(CACHE_FILE).json() as unknown
+    if (!raw || typeof raw !== 'object' || typeof (raw as Record<string, unknown>).checkedAt !== 'string') return null
+    const typed = raw as UpdateCache
+    const age = Date.now() - new Date(typed.checkedAt).getTime()
+    if (Number.isNaN(age)) return null
     if (age > CHECK_INTERVAL_MS) return null // stale
-    return raw
+    return typed
   } catch {
     return null
   }