fix(sync): remote URL allowlist, exit codes, progress output

src/commands/vault-sync.test.ts

@@ -50,10 +50,24 @@ const makeDeps = (over: Partial<VaultSyncDeps> = {}): { deps: VaultSyncDeps; log
 };
 
 describe('runVaultSync', () => {
-  it('reports "no syncable origin" for an unknown vault name', async () => {
-    const { deps, logs } = makeDeps();
-    await runVaultSync('missing', deps);
-    expect(logs.join()).toContain("No syncable origin configured for vault 'missing'");
+  it('errors when the named vault is not registered', async () => {
+    const { deps } = makeDeps();
+    await expect(runVaultSync('missing', deps)).rejects.toThrow("vault 'missing' not found");
+  });
+
+  it('reports "no syncable origin" for a registered vault with no external origin', async () => {
+    const { deps, logs } = makeDeps({
+      loadConfig: () => ({ version: 1, vaults: { local: { path: '/tmp/local' } } }),
+    });
+    await runVaultSync('local', deps);
+    expect(logs.join()).toContain("No syncable origin configured for vault 'local'");
+  });
+
+  it('prints an upfront count and a per-vault progress line', async () => {
+    const { deps, logs } = makeDeps({ daemonPort: async () => null });
+    await runVaultSync('v', deps);
+    expect(logs).toContain('Syncing 1 vault(s)...');
+    expect(logs).toContain('v...');
   });
 
   it('hints when no syncable vaults exist', async () => {

src/commands/vault-sync.ts

@@ -8,6 +8,7 @@ import { runSyncCycle, type SyncResult } from '../sync/cycle.js';
 import { createCouchSyncManager, type CouchSyncResult } from '../sync/couch/manager.js';
 import { couchTargets } from '../sync/couch/targets.js';
 import { syncTargets, type SyncTarget } from '../sync/planner.js';
+import { redactRemoteUrl } from '../sync/remote-url.js';
 
 export interface VaultSyncDeps {
   loadConfig: () => VaultsConfig;
@@ -46,7 +47,7 @@ const report = (log: (msg: string) => void, r: SyncRunResult): void => {
     log(`${r.vault} (account): ${describeCouch(r)}`);
     return;
   }
-  log(`${r.vault} -> ${r.remote}: ${describeGit(r)}`);
+  log(`${r.vault} -> ${redactRemoteUrl(r.remote)}: ${describeGit(r)}`);
   for (const c of r.conflicts) log(`  kept remote copy: ${c}`);
 };
 
@@ -59,6 +60,8 @@ export const runVaultSync = async (
   deps: VaultSyncDeps
 ): Promise<void> => {
   const config = deps.loadConfig();
+  // A named vault that is not registered is an error (mirrors `vault remove`), not a silent no-op.
+  if (name && !(config.vaults ?? {})[name]) throw new Error(`vault '${name}' not found`);
   const gitTargets = syncTargets(config).filter((t) => !name || t.vault === name);
   const couchVaults = couchTargets(config)
     .filter((t) => !name || t.vault === name)
@@ -71,14 +74,24 @@ export const runVaultSync = async (
     );
     return;
   }
+  const vaults = [...new Set([...gitTargets.map((t) => t.vault), ...couchVaults])];
+  deps.log(`Syncing ${vaults.length} vault(s)...`);
   const port = await deps.daemonPort();
   if (port !== null) {
-    const vaults = [...new Set([...gitTargets.map((t) => t.vault), ...couchVaults])];
-    for (const vault of vaults) report(deps.log, await deps.runViaDaemon(port, vault));
+    for (const vault of vaults) {
+      deps.log(`${vault}...`);
+      report(deps.log, await deps.runViaDaemon(port, vault));
+    }
     return;
   }
-  for (const target of gitTargets) report(deps.log, await deps.runGitInProcess(target));
-  for (const vault of couchVaults) report(deps.log, await deps.runCouchInProcess(vault));
+  for (const target of gitTargets) {
+    deps.log(`${target.vault}...`);
+    report(deps.log, await deps.runGitInProcess(target));
+  }
+  for (const vault of couchVaults) {
+    deps.log(`${vault}...`);
+    report(deps.log, await deps.runCouchInProcess(vault));
+  }
 };
 
 const resolveDaemonPort = async (): Promise<number | null> => {

src/commands/vault.test.ts

@@ -61,6 +61,22 @@ describe('vault add', () => {
     expect(h.logs.join()).toContain('(git)');
   });
 
+  it('rejects an unsafe --git transport-helper remote', async () => {
+    const h = makeDeps();
+    await expect(runVaultAdd('x', { git: 'ext::sh -c "id"' }, h.deps)).rejects.toThrow(
+      /unsafe git remote URL/
+    );
+    expect(h.get().vaults?.x).toBeUndefined();
+  });
+
+  it('redacts credentials in the echoed --git remote (stores the real URL)', async () => {
+    const h = makeDeps();
+    await runVaultAdd('work', { git: 'https://u:tok@h/w.git' }, h.deps);
+    expect(h.get().vaults?.work.origin?.[0]?.remote).toBe('https://u:tok@h/w.git');
+    expect(h.logs.join()).toContain('https://u:***@h/w.git');
+    expect(h.logs.join()).not.toContain('tok@');
+  });
+
   it('with no flag registers an account vault, writes the entry, then provisions', async () => {
     const h = makeDeps();
     await runVaultAdd('acct', {}, h.deps);
@@ -205,4 +221,15 @@ describe('vault list', () => {
     expect(out.a!.type).toBe('local');
     expect(out.acct!.type).toBe('account');
   });
+
+  it('redacts credentials in --json origin remotes', async () => {
+    const h = makeDeps();
+    await runVaultAdd('work', { git: 'https://u:tok@h/w.git' }, h.deps);
+    h.logs.length = 0;
+    runVaultList({ json: true }, h.deps);
+    expect(h.logs[0]).toContain('https://u:***@h/w.git');
+    expect(h.logs[0]).not.toContain('tok@');
+    // The on-disk value is untouched.
+    expect(h.get().vaults?.work.origin?.[0]?.remote).toBe('https://u:tok@h/w.git');
+  });
 });

src/commands/vault.ts

@@ -6,6 +6,7 @@ import {
   appendDiscoverIgnore,
   ensureVaultDir,
   formatVaultLine,
+  redactEntry,
   removeVault,
   vaultType,
 } from '../lib/vault-registry.js';
@@ -15,6 +16,7 @@ import {
   type ProvisionResult,
 } from '../lib/provision.js';
 import { loadVaultsConfig, mutateVaultsConfig, type LoadedVaults } from '../lib/vaults.js';
+import { assertSafeRemoteUrl, redactRemoteUrl } from '../sync/remote-url.js';
 import { defaultVaultSyncDeps, runVaultSync } from './vault-sync.js';
 
 export interface VaultDeps {
@@ -49,7 +51,10 @@ const buildEntry = (name: string, opts: VaultAddOptions): VaultEntry => {
     throw new Error('--path applies only to an account vault (drop --local and --git)');
   // A --git vault is a local working copy that syncs to an external git remote (path + origin);
   // the daemon commits/pushes and pulls it per its interval.
-  if (opts.git) return { path: `~/vaults/${name}`, origin: [{ remote: opts.git }], mcp: ['local'] };
+  if (opts.git) {
+    assertSafeRemoteUrl(opts.git);
+    return { path: `~/vaults/${name}`, origin: [{ remote: opts.git }], mcp: ['local'] };
+  }
   if (hasLocal) {
     const path = typeof opts.local === 'string' ? opts.local : `~/vaults/${name}`;
     return { path, mcp: ['local'] };
@@ -69,7 +74,8 @@ export const runVaultAdd = async (
   if (entry.path) deps.ensureDir(entry.path);
   const kind = vaultType(entry);
   const where = entry.path ?? entry.origin?.[0]?.remote ?? '';
-  const via = kind === 'git' && entry.origin?.length ? ` <- ${entry.origin[0]!.remote}` : '';
+  const via =
+    kind === 'git' && entry.origin?.length ? ` <- ${redactRemoteUrl(entry.origin[0]!.remote)}` : '';
   deps.log(chalk.green(`Added vault '${name}' (${kind}) -> ${where}${via}`));
   // Offline-first: the local entry is saved first; provisioning the account channel is never fatal.
   if (isAccountVault(entry)) deps.log((await deps.provision(name)).message);
@@ -105,8 +111,9 @@ export const runVaultList = (opts: { json?: boolean }, deps: VaultDeps = default
   const names = Object.keys(vaults);
   if (opts.json) {
     // Backward-compatible: still the name-keyed map, each entry annotated with its honest type.
+    // Origin remotes are redacted so a credentialed URL never leaks into machine output.
     const out = Object.fromEntries(
-      names.map((n) => [n, { ...vaults[n]!, type: vaultType(vaults[n]!) }])
+      names.map((n) => [n, { ...redactEntry(vaults[n]!), type: vaultType(vaults[n]!) }])
     );
     deps.log(JSON.stringify(out, null, 2));
     return;

src/lib/vault-registry.test.ts

@@ -9,6 +9,7 @@ import {
   appendDiscoverIgnore,
   ensureVaultDir,
   formatVaultLine,
+  redactEntry,
   removeVault,
   vaultType,
 } from './vault-registry.js';
@@ -137,6 +138,30 @@ describe('formatVaultLine', () => {
     expect(line).toContain('account');
     expect(line).not.toContain('<- agentage');
   });
+
+  it('redacts credentials in the echoed remote', () => {
+    const line = formatVaultLine('work', { origin: [{ remote: 'https://u:tok@h/w.git' }] });
+    expect(line).toContain('https://u:***@h/w.git');
+    expect(line).not.toContain('tok');
+  });
+});
+
+describe('redactEntry', () => {
+  it('redacts every origin remote without mutating the input', () => {
+    const entry = {
+      path: '/tmp/w',
+      origin: [{ remote: 'https://u:tok@h/w.git' }, { remote: 'git@h:me/w.git' }],
+    };
+    const out = redactEntry(entry);
+    expect(out.origin?.[0]?.remote).toBe('https://u:***@h/w.git');
+    expect(out.origin?.[1]?.remote).toBe('git@h:me/w.git');
+    expect(entry.origin[0].remote).toBe('https://u:tok@h/w.git');
+  });
+
+  it('returns origin-free entries as-is', () => {
+    const entry = { path: '/tmp/local' };
+    expect(redactEntry(entry)).toEqual(entry);
+  });
 });
 
 describe('ensureVaultDir', () => {

src/lib/vault-registry.ts

@@ -7,6 +7,7 @@ import {
   type VaultEntry,
   type VaultsConfig,
 } from '@agentage/memory-core';
+import { redactRemoteUrl } from '../sync/remote-url.js';
 import { isValidVaultName } from './vaults.schema.js';
 
 // Offline registry operations over the unified vaults.json (object map keyed by name). No
@@ -31,12 +32,23 @@ export const ensureVaultDir = (path: string): void => {
 
 export const formatVaultLine = (name: string, entry: VaultEntry): string => {
   const kind = vaultType(entry);
-  const where = entry.path ? expandPath(entry.path) : (entry.origin?.[0]?.remote ?? '');
+  const rawWhere = entry.path ? expandPath(entry.path) : (entry.origin?.[0]?.remote ?? '');
+  const where = entry.path ? rawWhere : redactRemoteUrl(rawWhere);
   // Only an external git remote is worth echoing; the account channel is implied by the type.
-  const remote = kind === 'git' && entry.origin?.length ? `  <- ${entry.origin[0]!.remote}` : '';
+  const remote =
+    kind === 'git' && entry.origin?.length
+      ? `  <- ${redactRemoteUrl(entry.origin[0]!.remote)}`
+      : '';
   return `${name.padEnd(16)} ${kind.padEnd(8)} ${where}${remote}`;
 };
 
+// A copy of `entry` with every origin remote redacted (credentials stripped) for display/JSON;
+// the on-disk value is never mutated.
+export const redactEntry = (entry: VaultEntry): VaultEntry =>
+  entry.origin?.length
+    ? { ...entry, origin: entry.origin.map((o) => ({ ...o, remote: redactRemoteUrl(o.remote) })) }
+    : entry;
+
 // Add an entry under `name`; the first vault added also becomes the `default`.
 export const addVault = (config: VaultsConfig, name: string, entry: VaultEntry): VaultsConfig => {
   if (!isValidVaultName(name)) throw new Error(`invalid vault name: ${JSON.stringify(name)}`);

src/sync/cycle.test.ts

@@ -57,6 +57,15 @@ describe('runSyncCycle', () => {
     expect(g(bare, ['show', 'main:notes/a.md'])).toContain('hello quokka');
   });
 
+  it('skips an unsafe transport-helper remote without touching git', async () => {
+    writeFile(work, 'a.md', 'x');
+    const result = await runSyncCycle(target({ path: work, remote: 'ext::sh -c "id"' }));
+    expect(result.ok).toBe(true);
+    expect(result.skipped).toBe('invalid-remote');
+    expect(result.pushed).toBe(false);
+    expect(existsSync(join(work, '.git'))).toBe(false);
+  });
+
   it('is a no-op on the second cycle (nothing to commit)', async () => {
     writeFile(work, 'a.md', 'x');
     await runSyncCycle(target({ path: work, remote: bare }));

src/sync/cycle.ts

@@ -4,6 +4,7 @@ import { dirname, join } from 'node:path';
 import { conflictName } from './conflict.js';
 import { createSyncGit, GitError, type GitErrorKind, type SyncGit } from './git-exec.js';
 import { type SyncTarget } from './planner.js';
+import { isSafeRemoteUrl } from './remote-url.js';
 
 export interface SyncResult {
   vault: string;
@@ -12,7 +13,7 @@ export interface SyncResult {
   committed: boolean; // a sync commit was made for a dirty working tree
   pushed: boolean;
   conflicts: string[]; // paths written as `<name>.conflict.md`
-  skipped?: 'lock' | 'busy';
+  skipped?: 'lock' | 'busy' | 'invalid-remote';
   reason?: GitErrorKind;
   error?: string;
 }
@@ -143,6 +144,8 @@ export const runSyncCycle = async (
     pushed: false,
     conflicts: [],
   };
+  // Defense in depth: the planner already drops unsafe remotes, but never add/set-url one here.
+  if (!isSafeRemoteUrl(target.remote)) return { ...base, ok: true, skipped: 'invalid-remote' };
   let committed = false;
   let conflicts: string[] = [];
   try {

src/sync/planner.test.ts

@@ -1,4 +1,4 @@
-import { describe, expect, it } from 'vitest';
+import { describe, expect, it, vi } from 'vitest';
 import { type VaultsConfig } from '@agentage/memory-core';
 import {
   autoSyncTargets,
@@ -78,6 +78,19 @@ describe('syncTargets', () => {
     expect(syncTargets(cfg({ v: { path: '/tmp/v', origin: [{ remote: '   ' }] } }))).toEqual([]);
   });
 
+  it('skips an unsafe transport-helper remote and warns, keeping other targets', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const targets = syncTargets(
+      cfg({
+        bad: { path: '/tmp/bad', origin: [{ remote: 'ext::sh -c "id"' }] },
+        good: { path: '/tmp/good', origin: [{ remote: 'git@h:me/g.git' }] },
+      })
+    );
+    expect(targets.map((t) => t.vault)).toEqual(['good']);
+    expect(warn).toHaveBeenCalledWith(expect.stringContaining("unsafe remote for vault 'bad'"));
+    warn.mockRestore();
+  });
+
   it('never picks up an account (agentage-origin) vault, even with a local path', () => {
     const config = cfg({
       acct: { path: '/tmp/acct', origin: [{ remote: 'agentage' }] },

src/sync/planner.ts

@@ -1,4 +1,5 @@
 import { expandPath, type VaultsConfig } from '@agentage/memory-core';
+import { isSafeRemoteUrl, redactRemoteUrl } from './remote-url.js';
 
 // The unified schema stores interval as a bare non-negative integer; sync treats it as SECONDS
 // (there is no minutes marker in the schema, so seconds is the least-surprising reading and keeps
@@ -39,6 +40,13 @@ export const syncTargets = (config: VaultsConfig): SyncTarget[] => {
     entry.origin.forEach((origin, index) => {
       const remote = origin.remote.trim();
       if (!remote || remote === RESERVED_REMOTE) return;
+      // One poisoned origin must not run code or kill the whole cycle: skip it with a warning.
+      if (!isSafeRemoteUrl(remote)) {
+        console.warn(
+          `agentage: skipping unsafe remote for vault '${vault}': ${redactRemoteUrl(remote)}`
+        );
+        return;
+      }
       out.push({
         vault,
         path: expandPath(entry.path as string),

src/sync/remote-url.test.ts

@@ -0,0 +1,63 @@
+import { describe, expect, it } from 'vitest';
+import { assertSafeRemoteUrl, isSafeRemoteUrl, redactRemoteUrl } from './remote-url.js';
+
+describe('isSafeRemoteUrl', () => {
+  it('allows conventional transports', () => {
+    expect(isSafeRemoteUrl('https://github.com/me/repo.git')).toBe(true);
+    expect(isSafeRemoteUrl('ssh://git@host/me/repo.git')).toBe(true);
+    expect(isSafeRemoteUrl('git://host/me/repo.git')).toBe(true);
+    expect(isSafeRemoteUrl('  https://host/repo.git  ')).toBe(true);
+  });
+
+  it('allows scp-like user@host:path', () => {
+    expect(isSafeRemoteUrl('git@github.com:me/repo.git')).toBe(true);
+    expect(isSafeRemoteUrl('user@host:path/to/repo')).toBe(true);
+  });
+
+  it('allows credentialed https (redaction handles display)', () => {
+    expect(isSafeRemoteUrl('https://user:token@host/repo.git')).toBe(true);
+  });
+
+  it('rejects transport-helper URLs (arbitrary command execution)', () => {
+    expect(isSafeRemoteUrl('ext::sh -c "id"')).toBe(false);
+    expect(isSafeRemoteUrl('fd::17')).toBe(false);
+    expect(isSafeRemoteUrl('transport::address')).toBe(false);
+    expect(isSafeRemoteUrl('user@host::ext')).toBe(false);
+  });
+
+  it('rejects a URL that could be read as a git option', () => {
+    expect(isSafeRemoteUrl('-oProxyCommand=evil')).toBe(false);
+  });
+
+  it('allows local file transports (no command execution)', () => {
+    expect(isSafeRemoteUrl('/srv/git/repo.git')).toBe(true);
+    expect(isSafeRemoteUrl('file:///srv/git/repo.git')).toBe(true);
+  });
+
+  it('rejects blank and non-transport strings', () => {
+    expect(isSafeRemoteUrl('')).toBe(false);
+    expect(isSafeRemoteUrl('   ')).toBe(false);
+    expect(isSafeRemoteUrl('C:\\repo')).toBe(false);
+  });
+});
+
+describe('assertSafeRemoteUrl', () => {
+  it('passes a safe URL and throws on an unsafe one', () => {
+    expect(() => assertSafeRemoteUrl('git@host:me/r.git')).not.toThrow();
+    expect(() => assertSafeRemoteUrl('ext::sh -c "id"')).toThrow('unsafe git remote URL');
+  });
+});
+
+describe('redactRemoteUrl', () => {
+  it('redacts the password in a credentialed URL, keeping the user', () => {
+    expect(redactRemoteUrl('https://user:token@host/repo.git')).toBe(
+      'https://user:***@host/repo.git'
+    );
+    expect(redactRemoteUrl('ssh://git:secret@host/repo')).toBe('ssh://git:***@host/repo');
+  });
+
+  it('leaves credential-free remotes untouched', () => {
+    expect(redactRemoteUrl('https://host/repo.git')).toBe('https://host/repo.git');
+    expect(redactRemoteUrl('git@host:me/r.git')).toBe('git@host:me/r.git');
+  });
+});