Skip to content

feat(vc): implement SSH commit signing (ENG-2002)#348

Open
hieuntg81 wants to merge 3 commits intomainfrom
proj/ssh-signing-key
Open

feat(vc): implement SSH commit signing (ENG-2002)#348
hieuntg81 wants to merge 3 commits intomainfrom
proj/ssh-signing-key

Conversation

@hieuntg81
Copy link
Copy Markdown
Collaborator

Summary

  • Problem: brv vc commit creates unsigned commits. There is no way to cryptographically sign commits made through ByteRover's isomorphic-git-based VC, limiting trust and traceability in collaborative workflows.
  • Why it matters: Signed commits prove authorship and are increasingly required by teams and platforms (e.g., GitHub verified badge). Without signing support, users who rely on brv vc cannot match the security posture they have with native git.
  • What changed:
    • New src/server/infra/ssh/ module: SSH key parser (Ed25519/RSA/ECDSA, OpenSSH + PEM formats), sshsig signer (PROTOCOL.sshsig-compliant envelope), SSH agent client (Unix domain socket protocol), and in-memory TTL key cache.
    • Three signing resolution paths in VcHandler.handleCommit: Path A -- ssh-agent (zero-prompt), Path B -- in-memory cache (passphrase-free after first use, 30 min TTL), Path C -- file-based parse (prompts for passphrase if encrypted, then caches).
    • New brv vc config keys: user.signingkey (path to SSH key), commit.sign (boolean), plus --import-git-signing flag to import from native git config.
    • New brv signing-key command group (add, list, remove) for managing signing keys on the Byterover IAM API.
    • New vc:signing-key transport event and SigningKeyHandler for daemon-side CRUD.
    • brv vc commit gains --sign / --no-sign flags with interactive passphrase retry (max 3 attempts).
    • resolveAuthor refactored: vc config -> auth token fallback (replaces duplicated buildAuthorHint).
  • What did NOT change (scope boundary):
    • No TUI/REPL changes -- signing is CLI-only for now.
    • No GPG support -- SSH only (gpg.format must be ssh or absent).
    • No signature verification -- this PR is sign-only; verification is a separate effort.
    • No changes to merge/pull commit signing (those remain unsigned).

Type of change

  • Bug fix
  • New feature
  • Refactor (no behavior change)
  • Documentation
  • Test
  • Chore (build, dependencies, CI)

Scope (select all touched areas)

  • TUI / REPL
  • Agent / Tools
  • LLM Providers
  • Server / Daemon
  • Shared (constants, types, transport events)
  • CLI Commands (oclif)
  • Hub / Connectors
  • Cloud Sync
  • CI/CD / Infra

Linked issues

  • Closes ENG-2002

Root cause (bug fixes only, otherwise write N/A)

N/A

Test plan

  • Coverage added:
    • Unit test
    • Integration test
    • Manual verification only
  • Test file(s):
    • test/unit/infra/ssh/ssh-key-parser.test.ts (169 lines)
    • test/unit/infra/ssh/ssh-agent-signer.test.ts (372 lines)
    • test/unit/infra/ssh/sshsig-signer.test.ts (79 lines)
    • test/unit/infra/ssh/signing-key-cache.test.ts (150 lines)
  • Key scenario(s) covered:
    • SSH key parsing: Ed25519/RSA/ECDSA key types, OpenSSH + PEM formats, passphrase-protected keys, ~ path resolution, probe (exists/needs-passphrase detection)
    • SSH agent: identity listing, signature request, fingerprint matching, agent unavailable fallback, timeout handling
    • sshsig: PROTOCOL.sshsig-compliant envelope construction, PEM armoring, SHA-512 hash-and-sign
    • Signing key cache: TTL expiry, cache invalidation, expired-entry sweep

User-visible changes

  • New CLI flags: brv vc commit --sign / --no-sign
  • New config keys: brv vc config user.signingkey <path>, brv vc config commit.sign true|false
  • New config flag: brv vc config --import-git-signing (imports from native git config)
  • New commands: brv signing-key add --key <path> [--title <label>], brv signing-key list, brv signing-key remove <id>
  • Interactive passphrase prompt when SSH key is encrypted and not in ssh-agent or cache
  • New error codes: PASSPHRASE_REQUIRED, SIGNING_KEY_NOT_CONFIGURED, SIGNING_KEY_NOT_FOUND

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording

Checklist

  • Tests added or updated and passing (npm test)
  • Lint passes (npm run lint)
  • Type check passes (npm run typecheck)
  • Build succeeds (npm run build)
  • Commits follow Conventional Commits format
  • Documentation updated (if applicable)
  • No breaking changes (or clearly documented above)
  • Branch is up to date with main

Risks and mitigations

  • Risk: Passphrase-decrypted key objects held in daemon memory (30 min TTL) could be exposed if the daemon process is compromised.
    • Mitigation: crypto.KeyObject is opaque (not extractable), passphrase itself is never stored, cache clears on daemon restart, per-key invalidation on config change.
  • Risk: ssh-agent socket communication uses raw Unix domain socket protocol -- malformed responses from a non-standard agent could cause unexpected behavior.
    • Mitigation: Strict buffer length checks, 5s timeout on agent operations, graceful fallback to file-based parsing if agent fails.
  • Risk: Branch includes unrelated commits from main history (release/3.1.0, adaptive knowledge, etc.) inflating the diff.
    • Mitigation: Rebase onto main before merge, or use squash merge scoped to the signing commit.

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