Skip to content

fix(git-ui): harden agent repository-operation facade (defense-in-depth)#1653

Merged
oscharko merged 1 commit into
feat/keiko-repository-centered-desktop-workflowfrom
feat/keiko-agent-facade-hardening-1577
Jun 28, 2026
Merged

fix(git-ui): harden agent repository-operation facade (defense-in-depth)#1653
oscharko merged 1 commit into
feat/keiko-repository-centered-desktop-workflowfrom
feat/keiko-agent-facade-hardening-1577

Conversation

@oscharko

Copy link
Copy Markdown
Contributor

Summary

Addresses the two LOW-severity, non-exploitable defense-in-depth hardening opportunities flagged during the independent acceptance-criteria audit of Epic #1571 on the agent repository-operation facade. The security-boundary review found the trust boundary intact; neither item is an acceptance failure. Both are tightened here with small, well-tested changes.

1. Bound the agent-facade idempotency cache (resource exhaustion, low)

POST /api/git/agent/operations kept its idempotency replay map (agentOperationsRoutes.ts) as an unbounded process-memory Map, keyed by projectId + idempotencyKey, with entries removed only on completion. A client streaming many distinct idempotency keys could grow it without limit.

  • Replaced the bare Map with an IdempotencyCache (bounded LRU + TTL): settled replay entries self-evict once over a size cap (DEFAULT_IDEMPOTENCY_MAX_ENTRIES = 1024) or past a TTL (DEFAULT_IDEMPOTENCY_TTL_MS = 10 min); each insert also opportunistically prunes expired entries so the map self-cleans even for keys never queried again.
  • In-flight (pending) reservations are exempt from TTL/LRU eviction, so the existing idempotency semantics are preserved exactly: replay-on-same-key, conflict-on-key-reuse (409), and reserve-before-settle all behave as before.
  • handleGitAgentOperationWithDelegate takes an optional injectable cache (default = module singleton) so eviction and TTL are deterministically testable.

2. Reject C0 control chars in pathspecs at the requestGuards layer (defense-in-depth, low, NOT exploitable)

isContainedPathspec (requestGuards.ts) rejected NUL, leading -//, Windows-absolute prefixes and .. segments, but not other C0 control characters (TAB/LF/CR).

  • Added GIT_DELIVERY_PATHSPEC_CONTROL_CHAR rejecting the full C0 control range plus DEL (which also subsumes the prior explicit NUL check), symmetric with the network-ref REF_CONTROL_CHAR guard used for remote aliases in syncRoutes.ts.
  • Not exploitable today — pathspecs are literalized as :(literal)<value> after a -- sentinel in git-mutation-adapter.ts. This is symmetric defense-in-depth at the boundary.

Tests

  • agentOperationsRoutes.test.ts: LRU eviction past the size cap, TTL expiry, prune-on-insert, in-flight-reservation exemption, concurrent-reservation retention, plus handler-path replay preservation, cache bounding over many keys, and re-delegation after TTL expiry.
  • New requestGuards.test.ts: asserts TAB/LF/CR/NUL/DEL pathspecs are rejected, valid pathspecs (incl. spaces) accepted, and pre-existing unsafe shapes still rejected.

Verification (local)

  • npx vitest run packages/keiko-server/src/gitDelivery14 files, 196 tests pass (was 183).
  • keiko-tools git suite → 16 files, 287 tests pass.
  • prettier --check, eslint (type-aware), and package-local tsc -b all clean on changed files.

Refs #1577 #1571

🤖 Generated with Claude Code

Two LOW-severity, non-exploitable defense-in-depth hardenings on the agent
repository-operation facade surfaced by the Epic #1571 security-boundary review.
The trust boundary was already intact; these tighten it symmetrically.

1. Bound the agent-facade idempotency cache. POST /api/git/agent/operations kept
   its idempotency replay map as an unbounded process-memory Map removed only on
   completion, so a client streaming many distinct idempotency keys could grow it
   without limit. Replace it with an IdempotencyCache (bounded LRU + TTL): settled
   replay entries self-evict past a size cap or after a TTL, while in-flight
   reservations are exempt from eviction so existing idempotency semantics
   (replay-on-same-key, conflict-on-key-reuse, reserve-before-settle) are
   preserved exactly. The handler takes an optional injectable cache for testing.

2. Reject C0 control chars in pathspecs at the requestGuards layer. isContainedPathspec
   now rejects TAB/LF/CR/NUL and all other C0 control / DEL chars, matching the
   network-ref REF_CONTROL_CHAR guard. Not exploitable today (pathspecs are
   literalized as :(literal)<value> after a "--" sentinel at the adapter) — this is
   symmetric defense-in-depth.

Adds unit tests for LRU + TTL eviction, in-flight-reservation exemption, replay
preservation, and TAB/LF/CR rejection. keiko-server gitDelivery suite 196 pass;
keiko-tools git suite 287 pass.

Refs #1577 #1571

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@oscharko oscharko merged commit e619143 into feat/keiko-repository-centered-desktop-workflow Jun 28, 2026
8 checks passed
@oscharko oscharko deleted the feat/keiko-agent-facade-hardening-1577 branch June 28, 2026 07:04
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