feat(git): stage / commit / push from the diff viewer (W4)
The walk-away endgame — review a diff on your phone, then land it without typing
git into a mobile terminal. MVP bounded to per-file stage/unstage, commit, and
push the current branch; discard/checkout/reset deliberately deferred (nothing here
mutates working-tree file contents).
- src/http/git-ops.ts (new): stageFiles/commit/push (execFile, no shell, never
throws). Path containment: every files[] entry realpath-contained under the repo,
argv after `--`, capped at diffMaxFiles. Commit message: empty/over-5000 → 400,
single -m argv. Push: current branch only — has-upstream → plain `git push`;
no-upstream + one remote → `git push -u <remote> <branch>`; 0 remotes → 400,
≥2 → 409, detached HEAD → 400. Remote AND branch read from the repo, never the
client. NEVER --force / +refspec. GIT_TERMINAL_PROMPT=0 + ssh BatchMode fail auth
fast (401). Errors classified to fixed safe strings (raw stderr never surfaced).
- POST /projects/git/{stage,commit,push} — all behind requireAllowedOrigin +
GIT_OPS_ENABLED kill-switch + per-IP rate limits (stage/commit 30/min, push 6/min)
+ isValidGitDir. public/diff.ts: per-file Stage/Unstage + a commit/push bar
(all text via textContent; re-loads the diff on success).
Verified: typecheck + build:web clean, git-ops tests 213 pass (unit covers escape
rejection / empty+overlong commit / push argv upstream-vs-no-upstream / classified
errors), full suite green at --test-timeout=30000.
This commit is contained in:
Yaojia Wang
322
test/http/git-ops.test.ts
Normal file
322
test/http/git-ops.test.ts
Normal file
@@ -0,0 +1,322 @@
|
||||
/**
|
||||
* test/http/git-ops.test.ts (w4-commit-push) — the git WRITE engine.
|
||||
*
|
||||
* Covers src/http/git-ops.ts:
|
||||
* - classifyGitError (pure: stderr → safe {status,error}, SEC-M10)
|
||||
* - validateRepoFiles (realpath containment: rejects escapes/absolute/flags)
|
||||
* - stageFiles (git add / restore --staged, real temp repos)
|
||||
* - commit (staged commit, empty/identity/length guards)
|
||||
* - push (upstream vs no-upstream argv, bare-remote real push)
|
||||
*
|
||||
* Pure functions are unit-tested; the git verbs run against real throwaway git
|
||||
* repos in os.tmpdir() (no network — a local bare repo is the push remote),
|
||||
* mirroring test/http/worktrees-create.test.ts.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeAll } from 'vitest'
|
||||
import fs from 'node:fs/promises'
|
||||
import os from 'node:os'
|
||||
import path from 'node:path'
|
||||
import { execFile } from 'node:child_process'
|
||||
import { promisify } from 'node:util'
|
||||
import {
|
||||
classifyGitError,
|
||||
validateRepoFiles,
|
||||
stageFiles,
|
||||
commit,
|
||||
push,
|
||||
} from '../../src/http/git-ops.js'
|
||||
|
||||
const execFileP = promisify(execFile)
|
||||
const TIMEOUT_MS = 15000
|
||||
const OPTS = { timeoutMs: TIMEOUT_MS, maxFiles: 300 }
|
||||
|
||||
let gitAvailable = false
|
||||
beforeAll(async () => {
|
||||
try {
|
||||
await execFileP('git', ['--version'])
|
||||
gitAvailable = true
|
||||
} catch {
|
||||
gitAvailable = false
|
||||
}
|
||||
})
|
||||
|
||||
/** Init a temp git repo with one commit; returns its REALPATH (macOS /var fix). */
|
||||
async function makeRepo(): Promise<string> {
|
||||
const dir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'gitops-')))
|
||||
const git = async (args: string[]): Promise<void> => {
|
||||
await execFileP('git', args, { cwd: dir })
|
||||
}
|
||||
await git(['init', '-q', '-b', 'main'])
|
||||
await git(['config', 'user.email', 't@t.local'])
|
||||
await git(['config', 'user.name', 'tester'])
|
||||
await git(['config', 'commit.gpgsign', 'false'])
|
||||
await fs.writeFile(path.join(dir, 'tracked.txt'), 'v1\n', 'utf8')
|
||||
await git(['add', '.'])
|
||||
await git(['commit', '-q', '-m', 'init'])
|
||||
return dir
|
||||
}
|
||||
|
||||
/** A bare repo to serve as a push `origin`. Returns its REALPATH. */
|
||||
async function makeBareRemote(): Promise<string> {
|
||||
const dir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'gitops-bare-')))
|
||||
await execFileP('git', ['init', '-q', '--bare', '-b', 'main', dir])
|
||||
return dir
|
||||
}
|
||||
|
||||
// ── classifyGitError (pure, SEC-M10) ────────────────────────────────────────────
|
||||
|
||||
describe('classifyGitError', () => {
|
||||
const cases: Array<[string, number]> = [
|
||||
['nothing to commit, working tree clean', 409],
|
||||
['no changes added to commit (use "git add")', 409],
|
||||
['Please tell me who you are', 400],
|
||||
[' ! [rejected] main -> main (non-fast-forward)', 409],
|
||||
['Updates were rejected because the tip of your current branch is behind\nfetch first', 409],
|
||||
['fatal: Authentication failed for \'https://github.com/x/y.git/\'', 401],
|
||||
["fatal: could not read Username for 'https://github.com': terminal prompts disabled", 401],
|
||||
['git@github.com: Permission denied (publickey).', 401],
|
||||
["fatal: Unable to create '/repo/.git/index.lock': File exists.", 409],
|
||||
['some totally unexpected failure', 500],
|
||||
]
|
||||
|
||||
it('maps each stderr signature to the expected status', () => {
|
||||
for (const [stderr, status] of cases) {
|
||||
expect(classifyGitError(stderr).status).toBe(status)
|
||||
}
|
||||
})
|
||||
|
||||
it('never leaks raw git stderr (no "fatal:" in any safe message)', () => {
|
||||
for (const [stderr] of cases) {
|
||||
expect(classifyGitError(stderr).error.toLowerCase()).not.toContain('fatal:')
|
||||
}
|
||||
})
|
||||
|
||||
it('is case-insensitive and tolerant of a non-string input', () => {
|
||||
expect(classifyGitError('NOTHING TO COMMIT').status).toBe(409)
|
||||
// @ts-expect-error — defensive: non-string must not throw
|
||||
expect(classifyGitError(undefined).status).toBe(500)
|
||||
})
|
||||
})
|
||||
|
||||
// ── validateRepoFiles (realpath containment) ────────────────────────────────────
|
||||
|
||||
describe('validateRepoFiles', () => {
|
||||
it('rejects an empty list', async () => {
|
||||
const repo = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'vrf-')))
|
||||
expect(await validateRepoFiles(repo, [], 300)).toBeNull()
|
||||
})
|
||||
|
||||
it('rejects more than maxFiles entries', async () => {
|
||||
const repo = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'vrf-')))
|
||||
expect(await validateRepoFiles(repo, ['a', 'b', 'c'], 2)).toBeNull()
|
||||
})
|
||||
|
||||
it('rejects non-string, absolute, and leading-dash entries', async () => {
|
||||
const repo = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'vrf-')))
|
||||
expect(await validateRepoFiles(repo, [123], 300)).toBeNull()
|
||||
expect(await validateRepoFiles(repo, [''], 300)).toBeNull()
|
||||
expect(await validateRepoFiles(repo, ['/etc/passwd'], 300)).toBeNull()
|
||||
expect(await validateRepoFiles(repo, ['-rf'], 300)).toBeNull()
|
||||
})
|
||||
|
||||
it('rejects a ../ traversal escape', async () => {
|
||||
const repo = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'vrf-')))
|
||||
expect(await validateRepoFiles(repo, ['../escape'], 300)).toBeNull()
|
||||
})
|
||||
|
||||
it('rejects the repo root itself (".")', async () => {
|
||||
const repo = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'vrf-')))
|
||||
expect(await validateRepoFiles(repo, ['.'], 300)).toBeNull()
|
||||
})
|
||||
|
||||
it('accepts in-repo relative paths (existing AND not-yet-on-disk / deleted)', async () => {
|
||||
const repo = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'vrf-')))
|
||||
await fs.writeFile(path.join(repo, 'exists.txt'), 'x', 'utf8')
|
||||
const ok = await validateRepoFiles(repo, ['exists.txt', 'sub/deleted.txt'], 300)
|
||||
expect(ok).toEqual(['exists.txt', 'sub/deleted.txt'])
|
||||
})
|
||||
|
||||
it('rejects a pre-planted symlink that escapes the repo after realpath (M2)', async () => {
|
||||
const repo = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'vrf-')))
|
||||
const outside = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'vrf-out-')))
|
||||
await fs.writeFile(path.join(outside, 'secret.txt'), 'top', 'utf8')
|
||||
await fs.symlink(path.join(outside, 'secret.txt'), path.join(repo, 'link.txt'))
|
||||
expect(await validateRepoFiles(repo, ['link.txt'], 300)).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
// ── stageFiles (real temp repos) ────────────────────────────────────────────────
|
||||
|
||||
describe('stageFiles', () => {
|
||||
it('stages the given files (git add) and unstages them (restore --staged)', async () => {
|
||||
if (!gitAvailable) return
|
||||
const repo = await makeRepo()
|
||||
await fs.writeFile(path.join(repo, 'tracked.txt'), 'v2\n', 'utf8') // modify tracked
|
||||
await fs.writeFile(path.join(repo, 'new.txt'), 'new\n', 'utf8') // add untracked
|
||||
|
||||
const staged = await stageFiles(repo, ['tracked.txt', 'new.txt'], true, OPTS)
|
||||
expect(staged).toMatchObject({ ok: true, staged: true, count: 2 })
|
||||
const cached = (await execFileP('git', ['diff', '--cached', '--name-only'], { cwd: repo })).stdout
|
||||
expect(cached).toContain('tracked.txt')
|
||||
expect(cached).toContain('new.txt')
|
||||
|
||||
const unstaged = await stageFiles(repo, ['tracked.txt'], false, OPTS)
|
||||
expect(unstaged).toMatchObject({ ok: true, staged: false, count: 1 })
|
||||
const cached2 = (await execFileP('git', ['diff', '--cached', '--name-only'], { cwd: repo })).stdout
|
||||
expect(cached2).not.toContain('tracked.txt')
|
||||
})
|
||||
|
||||
it('returns 404 for a non-git directory', async () => {
|
||||
const plain = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'gitops-plain-')))
|
||||
const res = await stageFiles(plain, ['x.txt'], true, OPTS)
|
||||
expect(res).toMatchObject({ ok: false, status: 404 })
|
||||
})
|
||||
|
||||
it('returns 400 for an escaping file path (never runs git)', async () => {
|
||||
if (!gitAvailable) return
|
||||
const repo = await makeRepo()
|
||||
const res = await stageFiles(repo, ['../../etc/passwd'], true, OPTS)
|
||||
expect(res).toMatchObject({ ok: false, status: 400 })
|
||||
})
|
||||
})
|
||||
|
||||
// ── commit (real temp repos) ────────────────────────────────────────────────────
|
||||
|
||||
describe('commit', () => {
|
||||
it('commits staged changes and returns a short SHA', async () => {
|
||||
if (!gitAvailable) return
|
||||
const repo = await makeRepo()
|
||||
await fs.writeFile(path.join(repo, 'tracked.txt'), 'v2\n', 'utf8')
|
||||
await execFileP('git', ['add', 'tracked.txt'], { cwd: repo })
|
||||
|
||||
const res = await commit(repo, 'second commit', { timeoutMs: TIMEOUT_MS, maxLen: 5000 })
|
||||
expect(res.ok).toBe(true)
|
||||
expect(typeof res.commit).toBe('string')
|
||||
const head = (await execFileP('git', ['rev-parse', 'HEAD'], { cwd: repo })).stdout.trim()
|
||||
expect(head.startsWith(res.commit as string)).toBe(true)
|
||||
})
|
||||
|
||||
it('returns 409 when nothing is staged', async () => {
|
||||
if (!gitAvailable) return
|
||||
const repo = await makeRepo()
|
||||
const res = await commit(repo, 'noop', { timeoutMs: TIMEOUT_MS, maxLen: 5000 })
|
||||
expect(res).toMatchObject({ ok: false, status: 409 })
|
||||
expect(res.error).not.toContain('fatal:')
|
||||
})
|
||||
|
||||
it('rejects an empty message with 400 (never runs git)', async () => {
|
||||
if (!gitAvailable) return
|
||||
const repo = await makeRepo()
|
||||
expect(await commit(repo, '', { timeoutMs: TIMEOUT_MS, maxLen: 5000 })).toMatchObject({
|
||||
ok: false,
|
||||
status: 400,
|
||||
})
|
||||
expect(await commit(repo, ' \n ', { timeoutMs: TIMEOUT_MS, maxLen: 5000 })).toMatchObject({
|
||||
ok: false,
|
||||
status: 400,
|
||||
})
|
||||
})
|
||||
|
||||
it('rejects an over-long message with 400', async () => {
|
||||
if (!gitAvailable) return
|
||||
const repo = await makeRepo()
|
||||
const res = await commit(repo, 'x'.repeat(51), { timeoutMs: TIMEOUT_MS, maxLen: 50 })
|
||||
expect(res).toMatchObject({ ok: false, status: 400 })
|
||||
})
|
||||
|
||||
it('returns 400 when the author identity is unset', async () => {
|
||||
if (!gitAvailable) return
|
||||
// Init WITHOUT any local identity, then hide the global/system config so no
|
||||
// identity resolves — git must fail with "Please tell me who you are" → 400.
|
||||
const repo = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'gitops-noid-')))
|
||||
await execFileP('git', ['init', '-q', '-b', 'main'], { cwd: repo })
|
||||
// useConfigOnly=true stops git auto-detecting an identity from the OS user, so
|
||||
// with no configured identity it hard-fails "Please tell me who you are".
|
||||
await execFileP('git', ['config', '--local', 'user.useConfigOnly', 'true'], { cwd: repo })
|
||||
await fs.writeFile(path.join(repo, 'f.txt'), 'x\n', 'utf8')
|
||||
await execFileP('git', ['add', 'f.txt'], { cwd: repo }) // stage on unborn HEAD
|
||||
|
||||
const prevG = process.env['GIT_CONFIG_GLOBAL']
|
||||
const prevS = process.env['GIT_CONFIG_SYSTEM']
|
||||
process.env['GIT_CONFIG_GLOBAL'] = '/dev/null'
|
||||
process.env['GIT_CONFIG_SYSTEM'] = '/dev/null'
|
||||
try {
|
||||
const res = await commit(repo, 'msg', { timeoutMs: TIMEOUT_MS, maxLen: 5000 })
|
||||
expect(res).toMatchObject({ ok: false, status: 400 })
|
||||
expect(res.error).not.toContain('fatal:')
|
||||
} finally {
|
||||
if (prevG === undefined) delete process.env['GIT_CONFIG_GLOBAL']
|
||||
else process.env['GIT_CONFIG_GLOBAL'] = prevG
|
||||
if (prevS === undefined) delete process.env['GIT_CONFIG_SYSTEM']
|
||||
else process.env['GIT_CONFIG_SYSTEM'] = prevS
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
// ── push (real bare-repo remote, no network) ────────────────────────────────────
|
||||
|
||||
describe('push', () => {
|
||||
it('first push sets upstream (-u) and the bare remote receives the ref', async () => {
|
||||
if (!gitAvailable) return
|
||||
const repo = await makeRepo()
|
||||
const bare = await makeBareRemote()
|
||||
await execFileP('git', ['remote', 'add', 'origin', bare], { cwd: repo })
|
||||
|
||||
const res = await push(repo, { timeoutMs: TIMEOUT_MS })
|
||||
expect(res).toMatchObject({ ok: true, branch: 'main', remote: 'origin' })
|
||||
const remoteHead = (await execFileP('git', ['--git-dir', bare, 'rev-parse', 'main'], {})).stdout.trim()
|
||||
const localHead = (await execFileP('git', ['rev-parse', 'HEAD'], { cwd: repo })).stdout.trim()
|
||||
expect(remoteHead).toBe(localHead)
|
||||
})
|
||||
|
||||
it('a second push (upstream now set) succeeds via a plain push', async () => {
|
||||
if (!gitAvailable) return
|
||||
const repo = await makeRepo()
|
||||
const bare = await makeBareRemote()
|
||||
await execFileP('git', ['remote', 'add', 'origin', bare], { cwd: repo })
|
||||
expect((await push(repo, { timeoutMs: TIMEOUT_MS })).ok).toBe(true)
|
||||
|
||||
await fs.writeFile(path.join(repo, 'tracked.txt'), 'v2\n', 'utf8')
|
||||
await execFileP('git', ['add', 'tracked.txt'], { cwd: repo })
|
||||
await execFileP('git', ['commit', '-q', '-m', 'second'], { cwd: repo })
|
||||
|
||||
const res = await push(repo, { timeoutMs: TIMEOUT_MS })
|
||||
expect(res).toMatchObject({ ok: true, branch: 'main', remote: 'origin' })
|
||||
const remoteHead = (await execFileP('git', ['--git-dir', bare, 'rev-parse', 'main'], {})).stdout.trim()
|
||||
const localHead = (await execFileP('git', ['rev-parse', 'HEAD'], { cwd: repo })).stdout.trim()
|
||||
expect(remoteHead).toBe(localHead)
|
||||
})
|
||||
|
||||
it('returns 400 for a detached HEAD', async () => {
|
||||
if (!gitAvailable) return
|
||||
const repo = await makeRepo()
|
||||
const sha = (await execFileP('git', ['rev-parse', 'HEAD'], { cwd: repo })).stdout.trim()
|
||||
await execFileP('git', ['checkout', '-q', sha], { cwd: repo }) // detach
|
||||
const res = await push(repo, { timeoutMs: TIMEOUT_MS })
|
||||
expect(res).toMatchObject({ ok: false, status: 400 })
|
||||
})
|
||||
|
||||
it('returns 400 when the repo has zero remotes', async () => {
|
||||
if (!gitAvailable) return
|
||||
const repo = await makeRepo()
|
||||
const res = await push(repo, { timeoutMs: TIMEOUT_MS })
|
||||
expect(res).toMatchObject({ ok: false, status: 400 })
|
||||
})
|
||||
|
||||
it('returns 409 with two remotes and no upstream (ambiguous)', async () => {
|
||||
if (!gitAvailable) return
|
||||
const repo = await makeRepo()
|
||||
const bare1 = await makeBareRemote()
|
||||
const bare2 = await makeBareRemote()
|
||||
await execFileP('git', ['remote', 'add', 'origin', bare1], { cwd: repo })
|
||||
await execFileP('git', ['remote', 'add', 'backup', bare2], { cwd: repo })
|
||||
const res = await push(repo, { timeoutMs: TIMEOUT_MS })
|
||||
expect(res).toMatchObject({ ok: false, status: 409 })
|
||||
})
|
||||
|
||||
it('returns 404 for a non-git directory', async () => {
|
||||
const plain = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'gitops-nrepo-')))
|
||||
expect(await push(plain, { timeoutMs: TIMEOUT_MS })).toMatchObject({ ok: false, status: 404 })
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user