Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ Restore the most recent successful local projection apply in an emergency:
skillsync rollback
```

The next sync applies the current vault assignments again.
Rollback restores files only. It does not change assignments or target settings.

Validate vault structure, registry hashes, JSON files, symlinks, and common credential formats without changing the vault:

Expand Down
2 changes: 1 addition & 1 deletion src/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -1813,7 +1813,7 @@ async function rollbackCommand(rest = []) {
deviceId: config.deviceId,
});
console.log(`Restored ${result.restored} skill projection${result.restored === 1 ? '' : 's'} from ${result.backupId}.`);
console.log('The next sync will apply the current vault assignments again.');
console.log('Rollback restores files only; it does not change assignments or target settings.');
}

async function doctor() {
Expand Down
10 changes: 8 additions & 2 deletions src/core/check.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import path from 'node:path';

import { assertSafePathSegment, hashDirectory } from './fs.js';

const IGNORED_ROOT_ENTRIES = new Set(['.git', '.skillsync-local']);
const GIT_ENTRY = '.git';
const LOCAL_STATE_ENTRY = '.skillsync-local';
const OWNERSHIP_MARKER = '.skillsync-owned.json';
const SECRET_PATTERNS = [
['private key', /-----BEGIN (?:RSA |EC |OPENSSH )?PRIVATE KEY-----/],
Expand Down Expand Up @@ -144,11 +145,16 @@ export async function checkVault(vaultPath, { verifyRegistry = true } = {}) {
throw validationError('Vault check', [`Vault is not a directory: ${root}`]);
}

const gitBacked = Boolean(await pathInfo(path.join(root, GIT_ENTRY)));
const localState = await pathInfo(path.join(root, LOCAL_STATE_ENTRY));
const inspected = await inspectTree(root, {
ignoreRootEntries: IGNORED_ROOT_ENTRIES,
ignoreRootEntries: new Set([GIT_ENTRY, LOCAL_STATE_ENTRY]),
rejectOwnershipMarkers: true,
});
const errors = [...inspected.errors];
if (gitBacked && localState) {
errors.push(`Reserved local state path in Git-backed vault: ${LOCAL_STATE_ENTRY}`);
}
const unsafeTree = inspected.errors.some((error) => (
error.startsWith('Symlinks are not allowed:')
|| error.startsWith('Unsupported file type:')
Expand Down
36 changes: 28 additions & 8 deletions src/core/device.js
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,9 @@ export async function planLinks({
}) {
const device = await loadLocalDevice(vaultPath, deviceId);
const registry = providedRegistry || await loadRegistry(vaultPath);
const approvedReplacements = new Set(replaceUnmanagedPaths.map((targetPath) => path.resolve(targetPath)));
const approvedReplacements = new Set(await Promise.all(
replaceUnmanagedPaths.map((targetPath) => canonicalTargetRoot(targetPath)),
));
const desiredByTarget = new Map();
for (const [skillName, targets] of Object.entries(device.installed)) {
assertSafePathSegment(skillName, 'Skill name');
Expand All @@ -942,7 +944,8 @@ export async function planLinks({
const targetGroups = new Map();
for (const [targetName, targetConfig] of Object.entries(device.targets)) {
const targetPath = path.resolve(expandHome(targetConfig.path));
const existing = targetGroups.get(targetPath);
const canonicalPath = await canonicalTargetRoot(targetPath);
const existing = targetGroups.get(canonicalPath);
if (existing && existing.mode !== targetConfig.mode) {
throw new Error(`Targets sharing ${targetPath} must use the same projection mode`);
}
Expand All @@ -954,12 +957,12 @@ export async function planLinks({
};
group.targetNames.push(targetName);
for (const skillName of desiredByTarget.get(targetName) || []) group.desired.add(skillName);
targetGroups.set(targetPath, group);
targetGroups.set(canonicalPath, group);
}

const targetRoots = [...targetGroups.keys()].sort();
for (const [index, root] of targetRoots.entries()) {
if (targetRoots.some((candidate, candidateIndex) => (
const canonicalRoots = [...targetGroups.keys()].sort();
for (const [index, root] of canonicalRoots.entries()) {
if (canonicalRoots.some((candidate, candidateIndex) => (
candidateIndex !== index && root.startsWith(candidate + path.sep)
))) {
throw new Error(`Configured skill targets cannot overlap: ${root}`);
Expand Down Expand Up @@ -1007,7 +1010,7 @@ export async function planLinks({
if (info.exists
&& !info.ownedSymlink
&& !info.ownedCopy
&& !approvedReplacements.has(path.resolve(destination))) {
&& !approvedReplacements.has(await canonicalTargetRoot(destination))) {
throw new Error(`Refusing to overwrite unmanaged target path: ${destination}`);
}
const sourceHash = registry.skills[skillName].hash;
Expand Down Expand Up @@ -1069,11 +1072,28 @@ export async function planLinks({
operations.sort((left, right) => left.destination.localeCompare(right.destination));
return {
deviceId,
roots: targetRoots,
roots: [...new Set(Object.values(device.targets)
.map((target) => path.resolve(expandHome(target.path))))].sort(),
operations,
};
}

async function canonicalTargetRoot(targetPath) {
const suffix = [];
let current = path.resolve(targetPath);
while (true) {
try {
return path.join(await realpath(current), ...suffix);
} catch (error) {
if (error.code !== 'ENOENT') throw error;
const parent = path.dirname(current);
if (parent === current) return path.resolve(targetPath);
suffix.unshift(path.basename(current));
current = parent;
}
}
}

export async function applyLinks(options) {
const roots = await approvedProjectionRoots(options.vaultPath, options.deviceId || defaultDeviceId());
let preparedPlan = null;
Expand Down
7 changes: 6 additions & 1 deletion src/core/transaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -449,10 +449,15 @@ export async function rollbackLatestFilesystemBackup({ vaultPath, deviceId, root
null,
).catch(() => null);
if (manifest?.state !== 'applied') continue;
if (!Array.isArray(manifest.roots)
|| manifest.roots.some((root) => typeof root !== 'string')) {
throw new Error(`Invalid projection backup manifest: ${entry.name}`);
}
// Local-only backups retain roots that were device-approved when captured.
return rollbackBackup({
vaultPath,
deviceId,
allowedRoots,
allowedRoots: normalizeRoots([...allowedRoots, ...manifest.roots]),
backupId: entry.name,
});
}
Expand Down
93 changes: 93 additions & 0 deletions test/core.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,32 @@ test('projection plans reject overlapping target roots', async () => {
);
});

test('projection plans reject overlapping target roots through symlink aliases', async () => {
const root = await tempDir();
const vault = path.join(root, 'vault');
const physicalTarget = path.join(root, 'physical-skills');
const aliasTarget = path.join(root, 'alias-skills');
await mkdir(path.join(physicalTarget, 'nested'), { recursive: true });
await symlink(physicalTarget, aliasTarget, 'dir');
await addTarget({
vaultPath: vault,
deviceId: 'macbook',
name: 'alias',
targetPath: aliasTarget,
});
await addTarget({
vaultPath: vault,
deviceId: 'macbook',
name: 'nested',
targetPath: path.join(physicalTarget, 'nested'),
});

await assert.rejects(
() => applyLinks({ vaultPath: vault, deviceId: 'macbook' }),
/Configured skill targets cannot overlap/,
);
});

test('installSkill tracks device-global installs without an agent target', async () => {
const root = await tempDir();
const vault = path.join(root, 'vault');
Expand Down Expand Up @@ -1360,6 +1386,38 @@ test('approved local skill replacement is backed up without weakening unmanaged
await assert.rejects(() => lstat(path.join(destination, '.skillsync-owned.json')), { code: 'ENOENT' });
});

test('approved local replacement works through equivalent target aliases', async () => {
const root = await tempDir();
const vault = path.join(root, 'vault');
const physicalTarget = path.join(root, 'physical-skills');
const firstAlias = path.join(root, 'first-skills');
const secondAlias = path.join(root, 'second-skills');
await makeSkill(physicalTarget, 'shared-skill', '# Local\n');
await symlink(physicalTarget, firstAlias, 'dir');
await symlink(physicalTarget, secondAlias, 'dir');
await makeSkill(path.join(vault, 'skills'), 'shared-skill', '# Vault\n');
await rebuildRegistry(vault);
await addTarget({ vaultPath: vault, deviceId: 'macbook', name: 'first', targetPath: firstAlias });
await addTarget({ vaultPath: vault, deviceId: 'macbook', name: 'second', targetPath: secondAlias });
await installSkill({
vaultPath: vault,
deviceId: 'macbook',
skillName: 'shared-skill',
targets: ['second'],
});

await applyLinks({
vaultPath: vault,
deviceId: 'macbook',
replaceUnmanagedPaths: [path.join(secondAlias, 'shared-skill')],
});

assert.equal(
await readFile(path.join(physicalTarget, 'shared-skill', 'SKILL.md'), 'utf8'),
'# Vault\n',
);
});

test('copy projections refuse local edits unless discarding them is explicit', async () => {
const root = await tempDir();
const vault = path.join(root, 'vault');
Expand Down Expand Up @@ -1551,6 +1609,27 @@ test('rollback restores the previous copy projection', async () => {
);
});

test('rollback restores projections from a removed target', async () => {
const root = await tempDir();
const vault = path.join(root, 'vault');
const target = path.join(root, 'codex-skills');
await makeSkill(path.join(vault, 'skills'), 'shared-skill', '# Shared\n');
await rebuildRegistry(vault);
await addTarget({ vaultPath: vault, deviceId: 'macbook', name: 'codex', targetPath: target });
await installSkill({
vaultPath: vault,
deviceId: 'macbook',
skillName: 'shared-skill',
targets: ['codex'],
});
await applyLinks({ vaultPath: vault, deviceId: 'macbook' });
await removeTargetAndPrune({ vaultPath: vault, deviceId: 'macbook', name: 'codex' });

await rollbackLinks({ vaultPath: vault, deviceId: 'macbook' });

assert.equal((await lstat(path.join(target, 'shared-skill'))).isSymbolicLink(), true);
});

test('vault checks reject stale registry entries, secrets, and symlinks', async () => {
const root = await tempDir();
const vault = path.join(root, 'vault');
Expand All @@ -1573,6 +1652,20 @@ test('vault checks reject stale registry entries, secrets, and symlinks', async
await assert.rejects(() => checkVault(vault), /Invalid JSON: state\/broken.json/);
});

test('vault checks reject local-state paths that Git can track', async () => {
const root = await tempDir();
const vault = path.join(root, 'vault');
await rebuildRegistry(vault);
await mkdir(path.join(vault, '.git'), { recursive: true });
await mkdir(path.join(vault, '.skillsync-local'), { recursive: true });
await writeFile(path.join(vault, '.skillsync-local', 'device.json'), '{}\n');

await assert.rejects(
() => checkVault(vault),
/Reserved local state path in Git-backed vault: \.skillsync-local/,
);
});

test('adding a skill rejects credentials before copying it into the vault', async () => {
const root = await tempDir();
const source = await makeSkill(root, 'unsafe-skill', '# Unsafe\n');
Expand Down