fix(sandbox): preserve workspace-local Skill symlinks

base-images/frameworks/sandbox/v1/README.md

@@ -12,10 +12,12 @@ The image contains:
 - `/opt/sealai/skill-bundle.mjs`: build, verify and local preparation implementation using the existing Node runtime.
 - `/usr/local/bin/sealai-prepare-skills`: fixed entry point for Brain, serialized with `flock`. Lock waiting and preparation share a 28-second budget (1-second kill grace), below Brain's 30-second RPC cap. Node, flock and timeout use absolute system paths; Node environment overrides are cleared.
 
-Run the entry point as the Devbox user after repository cloning; the workspace root must already exist. Chat has no repository clone and explicitly passes `--init-workspace`. Both modes prepare `/home/devbox/project/.agents/skills`, preserving unrelated Skills and `skills-lock.json`. Symlinks are rejected, and user files are type-checked without reading/hashing their contents.
+Run the entry point as the Devbox user after repository cloning; the workspace root must already exist. Chat has no repository clone and explicitly passes `--init-workspace`. Both modes prepare `/home/devbox/project/.agents/skills`, preserving unrelated Skills and `skills-lock.json`. Repository Skill symlinks are preserved verbatim when their destinations resolve inside the workspace, including aliases into `.claude/skills`. Dangling, cyclic and out-of-workspace links, special files, and symlinked workspace/Skill roots are rejected. Referenced directories are also checked, without reading/hashing user file contents. Image-owned bundles still reject all symlinks.
 
 Each replacement uses an exclusive `mkdtemp` transaction under `.sealai-skill-transactions`, outside `.agents` and on the workspace filesystem. If killed between renames, the next invocation restores the backup before creating any live Skill directory. Stages are never promoted during recovery because they may be incomplete. After publication, cleanup is best-effort; ambiguous recovery state fails closed and preserves files for inspection. Legacy PID-named backups are restored only when there is one backup and no live tree. This helper is not a security boundary against concurrent malicious filesystem mutation by the workspace owner, nor a power-loss durability guarantee.
 
+Recovery type-checks transaction trees without following symlinks, restores the previous tree to its live location, then validates link destinations before preparing again. Relative aliases must not be resolved from a temporary backup/stage path. Copying and cleanup do not follow links; replacing an alias whose name matches a bundled Skill leaves the alias destination untouched.
+
 The wrapper parses and validates success before printing schema, status, revision, bundle digest and Skill count as JSON. Empty/malformed output is failure. Failures use fixed codes, including preparation timeout, never file contents or raw exceptions. Missing or corrupt bundles fail closed. There is no npx/download fallback.
 
 ## Verification

base-images/frameworks/sandbox/v1/skill-bundle-regression.test.mjs

@@ -1,5 +1,5 @@
 import assert from 'node:assert/strict';
-import { mkdtemp, mkdir, readFile, realpath, rename, rm, symlink, writeFile } from 'node:fs/promises';
+import { lstat, mkdtemp, mkdir, readFile, readlink, realpath, rename, rm, symlink, writeFile } from 'node:fs/promises';
 import { spawnSync } from 'node:child_process';
 import os from 'node:os';
 import path from 'node:path';
@@ -22,6 +22,85 @@ async function fixture(t, names = ['sealos-deploy']) {
   return { root, source, bundle, workspace };
 }
 
+async function repositoryAlias(f, name = 'custom') {
+  const original = path.join(f.workspace, '.claude/skills', name);
+  const target = path.join(f.workspace, '.agents/skills');
+  await mkdir(original, { recursive: true });
+  await mkdir(target, { recursive: true });
+  await writeFile(path.join(original, 'SKILL.md'), 'repository skill');
+  const link = path.join(target, name);
+  const text = `../../.claude/skills/${name}`;
+  await symlink(text, link);
+  return { original, link, text };
+}
+
+test('Actual-style repository aliases retain their text and target across repeated preparation', async t => {
+  const f = await fixture(t);
+  const alias = await repositoryAlias(f);
+  // File aliases inside a referenced directory must also remain valid.
+  await symlink('SKILL.md', path.join(alias.original, 'instructions.md'));
+  for (let attempt = 0; attempt < 2; attempt++) {
+    assert.equal((await prepareBundle(f.bundle, f.workspace)).status, 'ready');
+    assert.equal(await readlink(alias.link), alias.text);
+    assert.equal(await realpath(alias.link), alias.original);
+    assert.equal(await readFile(path.join(alias.link, 'instructions.md'), 'utf8'), 'repository skill');
+    assert.match(await readFile(path.join(f.workspace, '.agents/skills/sealos-deploy/SKILL.md'), 'utf8'), /# Instructions/);
+  }
+});
+
+test('replacing a bundled Skill alias never overwrites the repository destination', async t => {
+  const f = await fixture(t);
+  const alias = await repositoryAlias(f, 'sealos-deploy');
+  await prepareBundle(f.bundle, f.workspace);
+  assert.equal((await lstat(alias.link)).isDirectory(), true);
+  assert.match(await readFile(path.join(alias.link, 'SKILL.md'), 'utf8'), /# Instructions/);
+  assert.equal(await readFile(path.join(alias.original, 'SKILL.md'), 'utf8'), 'repository skill');
+});
+
+for (const kind of ['external', 'indirect-external', 'dangling', 'link-cycle', 'directory-cycle']) {
+  test(`unsafe ${kind} alias fails without changing repository files`, async t => {
+    const f = await fixture(t);
+    const alias = await repositoryAlias(f);
+    const bad = path.join(alias.original, 'bad');
+    if (kind === 'external') await symlink(f.source, bad);
+    if (kind === 'indirect-external') {
+      await symlink(f.source, path.join(f.workspace, 'redirect'));
+      await symlink('../../../redirect', bad);
+    }
+    if (kind === 'dangling') await symlink('missing', bad);
+    if (kind === 'link-cycle') await symlink('bad', bad);
+    if (kind === 'directory-cycle') await symlink('.', bad);
+    await assert.rejects(prepareBundle(f.bundle, f.workspace), /workspace_symlink/);
+    assert.equal(await readlink(alias.link), alias.text);
+    assert.equal(await readFile(path.join(alias.original, 'SKILL.md'), 'utf8'), 'repository skill');
+    await assert.rejects(lstat(path.join(f.workspace, '.agents/skills/sealos-deploy')), { code: 'ENOENT' });
+  });
+}
+
+test('a symlink cannot replace the transaction backup root', async t => {
+  const f = await fixture(t);
+  const transaction = path.join(f.workspace, '.sealai-skill-transactions/txn-interrupted');
+  await mkdir(transaction, { recursive: true });
+  await symlink(f.source, path.join(transaction, 'backup'));
+  await assert.rejects(prepareBundle(f.bundle, f.workspace), /workspace_symlink/);
+  assert.equal(await readlink(path.join(transaction, 'backup')), f.source);
+  await assert.rejects(lstat(path.join(f.workspace, '.agents/skills')), { code: 'ENOENT' });
+});
+
+test('restored aliases are validated at their live location before a new replacement', async t => {
+  const f = await fixture(t);
+  const alias = await repositoryAlias(f);
+  const target = path.join(f.workspace, '.agents/skills');
+  const transaction = path.join(f.workspace, '.sealai-skill-transactions/txn-interrupted');
+  await mkdir(transaction, { recursive: true });
+  await rename(target, path.join(transaction, 'backup'));
+  await symlink(f.source, path.join(alias.original, 'escaped'));
+  await assert.rejects(prepareBundle(f.bundle, f.workspace), /workspace_symlink_outside/);
+  assert.equal(await readlink(alias.link), alias.text);
+  assert.equal(await readFile(path.join(alias.original, 'SKILL.md'), 'utf8'), 'repository skill');
+  await assert.rejects(lstat(path.join(target, 'sealos-deploy')), { code: 'ENOENT' });
+});
+
 test('all source Skills are bundled without a name or count allowlist', async t => {
   const f = await fixture(t, ['new-tool', 'another-tool']);
   assert.deepEqual((await verifyBundle(f.bundle)).skills, ['another-tool', 'new-tool']);
@@ -42,10 +121,13 @@ test('retry restores unrelated Skills from a legacy interrupted replacement', as
   const target = path.join(f.workspace, '.agents/skills');
   await mkdir(path.join(target, 'custom'));
   await writeFile(path.join(target, 'custom/keep'), 'user content');
+  const alias = await repositoryAlias(f, 'repo-alias');
   await rename(target, path.join(f.workspace, '.agents/skills-backup-99999'));
   await mkdir(path.join(f.workspace, '.agents/skills-stage-99999'));
   assert.equal((await prepareBundle(f.bundle, f.workspace)).status, 'ready');
   assert.equal(await readFile(path.join(target, 'custom/keep'), 'utf8'), 'user content');
+  assert.equal(await readlink(alias.link), alias.text);
+  assert.equal(await realpath(alias.link), alias.original);
 });
 
 test('deployment preparation requires an existing workspace', async t => {
@@ -69,6 +151,9 @@ for (const checkpoint of ['backup', 'published']) {
     const target = path.join(f.workspace, '.agents/skills');
     await mkdir(path.join(target, 'custom'));
     await writeFile(path.join(target, 'custom/keep'), 'user content');
+    const alias = await repositoryAlias(f, 'repo-alias');
+    // This link would be dangling if resolved at the backup/stage location.
+    await symlink('../custom/keep', path.join(target, 'custom/keep-alias'));
     const child = spawnSync(process.execPath, ['--input-type=module', '-e', `
       import fs from 'node:fs';
       import { syncBuiltinESMExports } from 'node:module';
@@ -84,6 +169,9 @@ for (const checkpoint of ['backup', 'published']) {
     assert.equal(child.signal, 'SIGKILL', child.stderr);
     assert.equal((await prepareBundle(f.bundle, f.workspace)).status, 'ready');
     assert.equal(await readFile(path.join(target, 'custom/keep'), 'utf8'), 'user content');
+    assert.equal(await readlink(alias.link), alias.text);
+    assert.equal(await realpath(alias.link), alias.original);
+    assert.equal(await readFile(path.join(target, 'custom/keep-alias'), 'utf8'), 'user content');
   });
 }
 

base-images/frameworks/sandbox/v1/skill-bundle.mjs

@@ -92,17 +92,44 @@ const exists = async entry => {
 };
 const cleanup = directory => rm(directory, { recursive: true, force: true }).catch(() => {});
 
+// Unlike image-owned bundles, repositories may alias Skills (e.g. to .claude).
+// Never read file bodies or dereference links while copying or removing a tree.
+// Recovery scans types only: relative links are meaningful at the live location,
+// not inside a transaction. Validate their destinations after restoring backup.
+async function workspaceFiles(root, workspace, ancestors = new Set()) {
+  if (!(await lstat(root)).isDirectory()) fail('workspace_symlink');
+  if (ancestors.has(root)) fail('workspace_symlink_cycle');
+  const parents = new Set([...ancestors, root]);
+  for (const entry of await readdir(root)) {
+    const file = path.join(root, entry);
+    const stat = await lstat(file);
+    if (stat.isSymbolicLink()) {
+      if (!workspace) continue;
+      let resolved;
+      try { resolved = await realpath(file); }
+      catch { fail('workspace_symlink_invalid'); }
+      const relative = path.relative(workspace, resolved);
+      if (relative === '..' || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative)) fail('workspace_symlink_outside');
+      const destination = await lstat(resolved);
+      if (destination.isDirectory()) await workspaceFiles(resolved, workspace, parents);
+      else if (!destination.isFile()) fail('workspace_special_file');
+    } else if (stat.isDirectory()) await workspaceFiles(file, workspace, parents);
+    else if (!stat.isFile()) fail('workspace_special_file');
+  }
+}
+
 // The caller holds flock. Backup presence is the recovery journal:
 // absent target + backup => restore old; present target => publication completed.
 async function recoverTransactions(state, target) {
   const transactions = (await readdir(state)).filter(name => name.startsWith('txn-'));
   if (transactions.length > 1) fail('recovery_ambiguous');
   for (const name of transactions) {
     const transaction = path.join(state, name);
-    await files(transaction, '', false);
+    await workspaceFiles(transaction);
     const backup = path.join(transaction, 'backup');
     if (!(await exists(target))) {
       if (!(await exists(backup))) fail('recovery_ambiguous');
+      await safeDirectory(backup);
       await rename(backup, target);
     }
     await cleanup(transaction);
@@ -117,12 +144,12 @@ async function recoverLegacy(agentRoot, target) {
   if (!backups.length && !stages.length) return;
   if (backups.length !== 1 || await exists(target)) fail('recovery_ambiguous');
   const backup = path.join(agentRoot, backups[0]);
-  await files(backup, '', false);
+  await workspaceFiles(backup);
   await rename(backup, target);
   // A stage may be incomplete. Never promote it over the previous user's tree.
   for (const name of stages) {
     const stage = path.join(agentRoot, name);
-    await files(stage, '', false);
+    await workspaceFiles(stage);
     await cleanup(stage);
   }
 }
@@ -138,12 +165,12 @@ export async function prepareBundle(bundle, workspace) {
   await recoverTransactions(state, target);
   await recoverLegacy(agentRoot, target);
   await safeDirectory(target, true);
-  await files(target, '', false); // Type scan only; do not hash user content.
+  await workspaceFiles(target, workspace);
   const transaction = await mkdtemp(path.join(state, 'txn-'));
   const stage = path.join(transaction, 'stage');
   const backup = path.join(transaction, 'backup');
   try {
-    await cp(target, stage, { recursive: true });
+    await cp(target, stage, { recursive: true, dereference: false, verbatimSymlinks: true });
     for (const name of manifest.skills) {
       await rm(path.join(stage, name), { recursive: true, force: true });
       await cp(path.join(bundle, 'skills', name), path.join(stage, name), { recursive: true });

base-images/frameworks/sandbox/v1/skill-bundle.test.mjs

@@ -81,12 +81,12 @@ test('repository symlinks cannot redirect Skill writes', async t => {
   assert.equal(await readFile(path.join(outside, 'keep'), 'utf8'), 'unchanged');
 });
 
-test('nested workspace symlinks and bundle symlinks are rejected', async t => {
+test('external workspace symlinks and bundle symlinks are rejected', async t => {
   const f = await fixture(t);
   const target = path.join(f.workspace, '.agents/skills/custom');
   await mkdir(target, { recursive: true });
   await symlink(f.skill, path.join(target, 'link'));
-  await assert.rejects(prepareBundle(f.bundle, f.workspace), /bundle_symlink/);
+  await assert.rejects(prepareBundle(f.bundle, f.workspace), /workspace_symlink_outside/);
   await symlink(f.skill, path.join(f.bundle, 'skills/link'));
   await assert.rejects(verifyBundle(f.bundle), /bundle_symlink/);
 });