kai/web-terminal
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
14f28e9d66bf57441686921fc019c1c94a9d860c
web-terminal/docs/plans/w4-commit-push.md

15 KiB
Raw Blame History

Stage / commit / push from the diff viewer

id: w4-commit-push — the highest-risk git write set. Scope is bounded to the MVP the task defines: per-file stage/unstage toggle + commit + push-current-branch. Discard / checkout / restore-working-tree are explicitly deferred (they are the only destructive-to-working-tree ops; nothing in this feature ever touches file contents on disk).

This mirrors the existing read-only diff channel (src/http/diff.ts) and the one existing git-write channel (src/http/worktrees.ts + the POST /projects/worktree route at src/server.ts:699–725). The server stays a byte-shuttle; these are out-of-band side-channel routes.

Contract

New routes (all in src/server.ts, all express.json, all requireAllowedOrigin + isValidGitDir)

Every route: requireAllowedOrigin(req, res) (CSRF, src/server.ts:352) → gitOpsEnabled gate (403 if off) → per-IP rate limit → validate path with the in-file isValidGitDir (three-prong: absolute + dir + .git, src/server.ts:123) → delegate to src/http/git-ops.ts. The delegate re-validates and realpath-contains every path (defense in depth — the route’s isValidGitDir does not follow symlinks).

Method + path Body Success Notes
POST /projects/git/stage { path: string, files: string[], stage?: boolean } 200 {ok:true, staged:boolean, count:number} stage default true → git add; false → unstage (git restore --staged). stage is an addition to the task’s bare {path,files[]} so one endpoint serves the toggle in both directions.
POST /projects/git/commit { path: string, message: string } 200 {ok:true, commit:string} (short SHA) Commits staged changes only. message capped at commitMsgMaxLen.
POST /projects/git/push { path: string } 200 {ok:true, branch:string, remote:string} Pushes the current branch to its existing upstream, else -u <sole-remote> <branch>. Remote/branch are derived server-side, never taken from the client.

Error shape (mirrors the worktree route at src/server.ts:724): res.status(result.status).json({ error: result.error }) where error is a safe message (never raw git stderr, SEC-M10 — see classifyWorktreeError, src/http/worktrees.ts:193).

Status codes: 400 invalid input / detached HEAD / no remote / identity unset; 403 disabled or bad Origin; 404 not a git dir; 409 nothing staged / non-fast-forward / index.lock held; 401 push auth required; 429 rate-limited; 500 unclassified; 200 success.

src/types.ts (coordination edit — frozen shared-contract source)

Add, next to CreateWorktreeResult (src/types.ts:485):

export interface GitOpResult {
  ok: boolean;
  status?: number;      // HTTP status on failure
  error?: string;       // SAFE message only (never raw git stderr)
  // success payloads (route-specific, all optional):
  staged?: boolean;     // stage: direction applied
  count?: number;       // stage: files affected
  commit?: string;      // commit: short SHA
  branch?: string;      // push: branch pushed
  remote?: string;      // push: remote pushed to
}

One interface keeps the coordination churn minimal (like CreateWorktreeResult). The FE narrows with an isGitOpResult guard mirroring isWorktreeResult (public/projects.ts:545).

src/config.ts env vars (4 new, mirroring the B3 worktree group at src/config.ts:371–378)

Env Field Default Purpose
GIT_OPS_ENABLED gitOpsEnabled: boolean true Master kill-switch (mirrors worktreeEnabled, src/config.ts:372). Off → all three routes 403.
GIT_OPS_TIMEOUT_MS gitOpsTimeoutMs: number 10_000 stage/commit exec timeout (mirrors DEFAULT_WORKTREE_TIMEOUT_MS, src/config.ts:65).
GIT_PUSH_TIMEOUT_MS gitPushTimeoutMs: number 120_000 push is network-bound → longer bound.
COMMIT_MSG_MAX_LEN commitMsgMaxLen: number 5_000 commit-message length cap.

Parsed with the existing parseBool / parseNonNegativeInt helpers and appended to the frozen object (src/config.ts:413–438). File-count cap for stage reuses the existing diffMaxFiles (src/config.ts:358, default 300) — no new var. src/types.ts Config interface gains the 4 fields (same coordination note the file already carries at src/config.ts:15).

Files to change

Path Change
src/http/git-ops.ts (new) The whole write engine. Mirrors diff.ts/worktrees.ts execFile pattern: no shell, timeout+maxBuffer, -- terminator, env:{...process.env, GIT_TERMINAL_PROMPT:'0'}. Exports (all never-throw, return GitOpResult): stageFiles(repoPath, files, stage, opts), commit(repoPath, message, opts), push(repoPath, opts), plus pure classifyGitError(stderr): {status,error} and validateRepoFiles(repoRealPath, files): string[] | null.
src/http/git-path.ts (new, small) Extracts the containment helper so it isn’t duplicated: resolveRealPath(target) (copy of worktrees.ts:118) + isContained(realBase, realCandidate): boolean. Used by git-ops.ts. (Optional follow-up: refactor worktrees.ts:141 computeWorktreeDir onto it — deferred, out of this task’s lane.)
src/types.ts Coordination edit: add GitOpResult (above) + 4 Config fields.
src/config.ts Parse the 4 new env vars; append to the frozen config object.
src/server.ts Register the 3 routes after the worktree route (:725). Add two module-level rate constants + two limiters via createRateLimiter (:109). Reuse in-file isValidGitDir, requireAllowedOrigin, sanitizeForLog (:162) for the commit/push audit log. Import from ./http/git-ops.js.
public/diff.ts mountDiffViewer (:253) gains: per-file Stage/Unstage toggle button in each file row, and a bottom commit/push bar (message <textarea> + Commit + Push). Add optional onToggleStage to the render path; add POST helpers postStage/postCommit/postPush. All text via textContent/el() (SEC-H4, :139 note). Re-loadDiff() after any successful write.
public/projects.ts No logic change required — buildDiffSection (:612) already mounts mountDiffViewer and gets the new UI for free. Only add CSS-class hooks if styling inline.
public/style.css (or wherever df-* classes live) Styles for df-file-stage, df-commitbar, df-commit-msg, df-commit-btn, df-push-btn, df-op-error, df-op-busy.

TDD steps (ordered)

Backend uses node env + real throwaway git repos in os.tmpdir() exactly like test/http/diff.test.ts:277 and test/http/worktrees-create.test.ts:42. FE uses jsdom + mocked fetch like test/diff.test.ts.

Pure classifier first (fast, deterministic):

  1. test/http/git-ops.test.ts → describe('classifyGitError'): feed canned stderr strings, assert {status,error} and that error never contains fatal: (SEC-M10, mirror worktrees-create.test.ts:193). Cases: nothing to commit→409; Please tell me who you are→400; ! [rejected]/non-fast-forward/fetch first→409; could not read from Username/Authentication failed/terminal prompts disabled/Permission denied (publickey)→401; index.lock→409; unknown→500. → then implement classifyGitError.
  2. describe('validateRepoFiles'): rejects [], absolute paths, leading--, ../escape, > diffMaxFiles entries; accepts in-repo relative paths (incl. a deleted file whose path no longer exists on disk — resolveRealPath resolves the existing prefix). Symlink-escape: pre-plant a symlink inside the repo pointing outside, assert rejection (mirror worktrees-create.test.ts:137). → implement validateRepoFiles on git-path.ts.

Integration against real repos: 3. describe('stageFiles'): init repo + commit; modify a tracked file + add an untracked file; stageFiles(repo,[file],true) → assert git diff --cached --name-only lists them; stage:false → assert unstaged. Assert non-git path → {ok:false,status:404}. → implement stageFiles. 4. describe('commit'): stage a change → commit(repo,'msg') → {ok:true, commit} and git log -1 --format=%H starts with it. Empty index → {ok:false,status:409}. Repo with user.name unset → 400. Message length > cap → 400 (route-level; test the length guard where it lives). → implement commit. 5. describe('push'): create a bare repo as origin (git init --bare), git remote add origin, first push sets upstream via -u; assert bare repo received the ref (git --git-dir=<bare> rev-parse <branch>). Detached HEAD → 400. Zero remotes → 400. Two remotes + no upstream → 409. Second push (upstream now set) → plain git push. → implement push.

Config + route wiring: 6. test/config.test.ts: the 4 new vars default correctly and parse/validate (reuse existing patterns). 7. test/integration/server.test.ts (extend, using the startServer + fetch + Origin pattern at :783): for each of the 3 routes — foreign Origin → 403, no Origin → 403 (default-deny, mirror :778), non-git path → 404, missing field → 400, gitOpsEnabled:false → 403, and a happy-path 200 against a temp repo (push against a temp bare remote). Confirm error bodies carry no fatal:.

Frontend (jsdom): 8. test/diff.test.ts (extend): stub global.fetch. Assert each file row renders a Stage button on the Working view and Unstage on the Staged view; clicking POSTs to /projects/git/stage with the right {path,files,stage} body and re-fetches. Assert the commit bar disables Commit when the message is empty, POSTs /projects/git/commit, and shows a failure via textContent (SEC-H4 — assert zero innerHTML, mirror the file’s existing security assertions). Assert Push POSTs /projects/git/push and reflects busy/disabled state. → implement the mountDiffViewer changes to pass.

Coverage: the pure classifyGitError/validateRepoFiles + real-repo integration cover git-ops.ts branches; steps 7–8 cover the new server + diff.ts lines. Keeps the 80% gate.

Edge cases & failure modes

  • Nothing staged → commit: git exits non-zero (nothing to commit) → 409 "Nothing staged to commit." (not a 500).
  • Author identity unset: commit fails (Please tell me who you are) → 400 with a safe hint. Do not auto-inject a fake identity.
  • Unborn HEAD (no commits yet): unstage via git restore --staged errors → classify to a safe 409/400; commit still works (creates the first commit). Documented limitation.
  • Detached HEAD on push: git rev-parse --abbrev-ref HEAD → HEAD → refuse 400 "Cannot push a detached HEAD."
  • No upstream, one remote: git push -u <remote> <branch>. No upstream, ≥2 remotes: 409 "Set an upstream first." Zero remotes: 400.
  • Non-fast-forward / remote ahead: ! [rejected] → 409 "Push rejected — pull/rebase first." (never force-push).
  • Credential prompt hang: GIT_TERMINAL_PROMPT=0 (+ optional GIT_SSH_COMMAND='ssh -o BatchMode=yes') makes auth fail fast → 401 instead of hanging; the exec timeout is the backstop.
  • index.lock held (concurrent op): → 409 "Another git operation is in progress." (Optional hardening: a per-realpath in-memory promise-chain mutex in server.ts to serialize writes; git’s own lock is the correctness backstop, so this is MEDIUM, not required for MVP.)
  • Deleted file staged: path absent on disk → resolveRealPath resolves the existing prefix and re-appends the basename; git add -- <path> records the deletion. Renamed file: FE sends both oldPath and newPath so the deletion+addition both stage.
  • Huge files[]: capped at diffMaxFiles → 400.
  • maxBuffer overflow / timeout: caught, returned as a safe 500 (never a thrown rejection — house style, diff.ts:302).
  • Commit message with newlines / leading - / emoji: safe — passed as the single value of -m via argv (no shell), length-capped, never a pathspec.
  • Write succeeds but re-fetch fails: FE shows the diff error state but the write already happened; surface a non-blocking notice.

Security

  • Origin/CSRF: all 3 routes call requireAllowedOrigin (src/server.ts:352) — this is a write channel, unlike read-only /projects/diff which has no guard. Tested for foreign-Origin and missing-Origin (default-deny).
  • No shell, ever: execFile('git', argv, …) only — mirrors diff.ts:296 / worktrees.ts:242. Untrusted strings (message, file paths) are argv elements, never interpolated. -- terminates options before any pathspec so a path can’t become a flag; file paths additionally rejected if they start with -.
  • Path containment (highest-risk): isValidGitDir at the route (absolute+dir+.git) plus realpath-based containment of the repo and of every files[] entry inside git-ops.ts (resolveRealPath + startsWith(realBase+sep), the M2 pattern from worktrees.ts:141). Defeats ../ traversal and pre-planted-symlink escapes even though the diff route only does the lighter three-prong check.
  • Server-derived remote/branch: push never accepts a remote or refspec from the client — both are read back from the repo — eliminating arg-injection and push-to-arbitrary-URL risk.
  • Safe error classification: classifyGitError maps stderr substrings to safe messages; raw git stderr is never returned (SEC-M10). Tested that responses contain no fatal:.
  • Rate limiting: reuse createRateLimiter (src/server.ts:109, 60 s window) — gitWriteLimiter (stage+commit) ≈ 30/min/IP, gitPushLimiter ≈ 6/min/IP; over-limit → 429. Fixed policy constants beside the existing ones (src/server.ts:74–76).
  • Audit log: commit/push log actor+path via sanitizeForLog (src/server.ts:162, control-char strip + truncate), like the worktree audit at :714.
  • Kill-switch: gitOpsEnabled=false disables all three (mirrors worktreeEnabled, src/server.ts:701) for locked-down deployments.
  • Body size: express.json({ limit: '4kb' }) for commit/push, '64kb' for stage (large files[]) — matching the existing routes’ caps.
  • FE injection: all rendered git output/errors via textContent/el() — zero innerHTML (SEC-H4, asserted in test/diff.test.ts).

Effort & dependencies

  • Estimate: ~3–4 days. Backend git-ops.ts + classifier + containment ≈ 1.5 d; route wiring + config + integration tests ≈ 0.75 d; FE toggles + commit/push bar + jsdom tests ≈ 1 d; polish/edge-cases ≈ 0.5 d.
  • Depends on: the existing B1 diff channel (src/http/diff.ts, public/diff.ts:253 mountDiffViewer) and B3 worktree channel (src/http/worktrees.ts) — both shipped; this reuses their execFile + realpath-containment + error-classification patterns directly. No new dependency on other roadmap items.
  • Shares infra with w4-worktree-remove (task #12): both are git-write routes under requireAllowedOrigin + classifyGitError; landing the shared git-path.ts + classifyGitError here de-risks that one. Extractable-later: a generic runGitWrite() wrapper could later back worktrees.ts too (deferred — out of lane).
  • Unlocks: an in-browser commit loop for the walk-away workflow (stage → commit → push from a phone), and a natural home for a future "recent commits" chip (task #11).