Skip to content

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

Merged
vreshch merged 1 commit into
masterfrom
fix/sync-remote-allowlist
Jul 7, 2026
Merged

fix(sync): remote URL allowlist, exit codes, progress output#242
vreshch merged 1 commit into
masterfrom
fix/sync-remote-allowlist

Conversation

@vreshch

@vreshch vreshch commented Jul 7, 2026

Copy link
Copy Markdown
Member

What

Hardens vault git sync against poisoned remotes and improves vault sync UX.

  • Remote URL scheme allowlist (security). New src/sync/remote-url.ts (isSafeRemoteUrl / assertSafeRemoteUrl). Allows only https://, ssh://, git://, file://, scp-like user@host:path, and absolute local paths. Rejects transport helpers (ext::, fd::, transport::, any <helper>::) and any URL starting with - - these let git run arbitrary commands on auto-sync. Enforced at vault add --git (friendly error, exit 1) and in the sync planner (skips the origin with a one-line warning so one bad origin can't kill the cycle), plus a minimal defensive gate in cycle.ts that returns skipped: 'invalid-remote' before any remote add/set-url.
  • Credential redaction. user:pass@host in a remote is redacted to user:***@host in all human output (formatVaultLine, vault add echo, vault sync report) and --json (vault list). The stored value is untouched.
  • vault sync <nonexistent> exit code. A name not in the registry now errors vault '<name>' not found and exits 1 (mirrors vault remove). A registered vault with no syncable origin keeps its no-op message and exit 0.
  • Progress feedback. vault sync prints an upfront Syncing <N> vault(s)... line and a per-vault <name>... line before each sync starts, so it no longer looks hung on slow remotes.

Why

Git honors transport-helper URLs as remotes, so a poisoned vaults.json origin was arbitrary command execution when the daemon auto-syncs. Printed remotes could also leak credentials, and vault sync had a misleading exit code + no progress output.

Notes

  • file:// and absolute local paths are allowed alongside the scheme allowlist: they are legitimate git transports that cannot execute commands (and the crash-recovery cycle tests sync to local bare repos).
  • No changes to daemon, memory, file-lock, or api/oauth files. cycle.ts change is a single validation gate; its crash-recovery tests are untouched.

Verified: npm run verify green locally (357 tests); npm run test:coverage exit 0, thresholds met.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🎉 PR Validation ✅ PASSED

Commit: 26f7758e0b787f0bc3ae8ea2d360bf673f45b1d4
Branch: fix/sync-remote-allowlist

Checks:

  • ✅ Release guard (no version/changelog changes)
  • ✅ Dependencies installed
  • ✅ Type check passed
  • ✅ Linting passed
  • ✅ Format check passed
  • ✅ Tests + coverage passed
  • ✅ Build successful

Ready to merge!


🔗 View workflow run
⏰ Generated at: 2026-07-07T22:05:25.495Z

@vreshch
vreshch marked this pull request as ready for review July 7, 2026 22:09
@vreshch
vreshch merged commit 71bedf0 into master Jul 7, 2026
2 checks passed
@vreshch
vreshch deleted the fix/sync-remote-allowlist branch July 7, 2026 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant