diff --git a/.changeset/fix-parse-git-status.md b/.changeset/fix-parse-git-status.md new file mode 100644 index 0000000000..ef991cf78e --- /dev/null +++ b/.changeset/fix-parse-git-status.md @@ -0,0 +1,9 @@ +--- +"e2b": patch +"@e2b/python-sdk": patch +--- + +fix(git): prevent crash on malformed branch lines and false detached HEAD detection + +- Use `split("...", 1)` / `indexOf("...")` to avoid `ValueError` / incorrect destructuring when the branch line contains multiple `...` sequences (#1371) +- Remove overly broad `"detached" in raw_branch` / `rawBranch.includes('detached')` check that misidentified branches with "detached" in their name as detached HEAD (#1373) diff --git a/packages/js-sdk/src/sandbox/git/utils.ts b/packages/js-sdk/src/sandbox/git/utils.ts index 6df7fbc888..2964448074 100644 --- a/packages/js-sdk/src/sandbox/git/utils.ts +++ b/packages/js-sdk/src/sandbox/git/utils.ts @@ -381,14 +381,14 @@ export function parseGitStatus(output: string): GitStatus { aheadStart === -1 ? undefined : branchInfo.slice(aheadStart + 2, -1) const normalizedBranch = normalizeBranchName(branchPart) const rawBranch = branchPart - const isDetached = - rawBranch.startsWith('HEAD (detached at ') || - rawBranch.includes('detached') + const isDetached = rawBranch.startsWith('HEAD (detached at ') if (isDetached || normalizedBranch.startsWith('HEAD')) { detached = true } else if (normalizedBranch.includes('...')) { - const [branch, upstreamBranch] = normalizedBranch.split('...') + const idx = normalizedBranch.indexOf('...') + const branch = normalizedBranch.slice(0, idx) + const upstreamBranch = normalizedBranch.slice(idx + 3) currentBranch = branch || undefined upstream = upstreamBranch || undefined } else { diff --git a/packages/js-sdk/tests/sandbox/git/parseGitStatus.test.ts b/packages/js-sdk/tests/sandbox/git/parseGitStatus.test.ts new file mode 100644 index 0000000000..ef885976ee --- /dev/null +++ b/packages/js-sdk/tests/sandbox/git/parseGitStatus.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, test } from 'vitest' + +import { parseGitStatus } from '../../../src/sandbox/git/utils.js' + +describe('parseGitStatus', () => { + test('malformed branch line with multiple "..." does not crash (issue #1371)', () => { + const output = '## feat...v2...origin/feat...v2\n' + // Should not throw; exact parsing is best-effort + const status = parseGitStatus(output) + expect(status.currentBranch).toBeDefined() + expect(status.detached).toBe(false) + }) + + test('upstream branch containing "detached" is not misidentified (issue #1373)', () => { + const output = '## my-branch...origin/detached-work\n' + const status = parseGitStatus(output) + expect(status.currentBranch).toBe('my-branch') + expect(status.upstream).toBe('origin/detached-work') + expect(status.detached).toBe(false) + }) + + test('branch literally named "detached" is not treated as detached HEAD', () => { + const output = '## detached\n' + const status = parseGitStatus(output) + expect(status.currentBranch).toBe('detached') + expect(status.detached).toBe(false) + }) + + test('real detached HEAD is still detected', () => { + const output = '## HEAD (detached at abc1234)\n' + const status = parseGitStatus(output) + expect(status.detached).toBe(true) + }) + + test('branch name containing "detached" substring is not misidentified', () => { + const output = '## fix-detached-bug...origin/fix-detached-bug\n' + const status = parseGitStatus(output) + expect(status.currentBranch).toBe('fix-detached-bug') + expect(status.upstream).toBe('origin/fix-detached-bug') + expect(status.detached).toBe(false) + }) + + test('no commits yet parses as branch, not detached', () => { + const output = '## No commits yet on main\n' + const status = parseGitStatus(output) + expect(status.currentBranch).toBe('main') + expect(status.detached).toBe(false) + }) + + test('HEAD (no branch) is treated as detached', () => { + const output = '## HEAD (no branch)\n' + const status = parseGitStatus(output) + expect(status.detached).toBe(true) + }) + + test('simple branch without upstream', () => { + const output = '## feature-branch\n' + const status = parseGitStatus(output) + expect(status.currentBranch).toBe('feature-branch') + expect(status.upstream).toBeUndefined() + expect(status.detached).toBe(false) + }) + + test('empty output returns clean status', () => { + const status = parseGitStatus('') + expect(status.currentBranch).toBeUndefined() + expect(status.detached).toBe(false) + expect(status.fileStatus).toEqual([]) + }) + + test('normal branch with upstream parses correctly', () => { + const output = '## main...origin/main [ahead 1, behind 2]\n' + const status = parseGitStatus(output) + expect(status.currentBranch).toBe('main') + expect(status.upstream).toBe('origin/main') + expect(status.ahead).toBe(1) + expect(status.behind).toBe(2) + expect(status.detached).toBe(false) + }) +}) diff --git a/packages/python-sdk/e2b/sandbox/_git/parse.py b/packages/python-sdk/e2b/sandbox/_git/parse.py index e6cea693af..a8ad3bbf00 100644 --- a/packages/python-sdk/e2b/sandbox/_git/parse.py +++ b/packages/python-sdk/e2b/sandbox/_git/parse.py @@ -125,14 +125,12 @@ def parse_git_status(output: str) -> GitStatus: ahead_part = None if ahead_start == -1 else branch_info[ahead_start + 2 : -1] normalized_branch = _normalize_branch_name(branch_part) raw_branch = branch_part - is_detached = raw_branch.startswith("HEAD (detached at ") or ( - "detached" in raw_branch - ) + is_detached = raw_branch.startswith("HEAD (detached at ") if is_detached or normalized_branch.startswith("HEAD"): detached = True elif "..." in normalized_branch: - branch, upstream_branch = normalized_branch.split("...") + branch, upstream_branch = normalized_branch.split("...", 1) current_branch = branch or None upstream = upstream_branch or None else: diff --git a/packages/python-sdk/tests/test_parse_git_status.py b/packages/python-sdk/tests/test_parse_git_status.py new file mode 100644 index 0000000000..855957aaf1 --- /dev/null +++ b/packages/python-sdk/tests/test_parse_git_status.py @@ -0,0 +1,88 @@ +from e2b.sandbox._git.parse import parse_git_status + + +def test_split_does_not_crash_with_extra_dots(): + """Malformed branch lines with multiple '...' should not crash (issue #1371).""" + output = "## feat...v2...origin/feat...v2\n" + # Should not raise ValueError; exact parsing is best-effort + status = parse_git_status(output) + assert status.current_branch is not None + assert status.detached is False + + +def test_upstream_branch_containing_detached(): + """Branches tracking an upstream with 'detached' in the name should not + be misidentified as detached HEAD (issue #1373).""" + output = "## my-branch...origin/detached-work\n" + status = parse_git_status(output) + assert status.current_branch == "my-branch" + assert status.upstream == "origin/detached-work" + assert status.detached is False + + +def test_branch_named_detached(): + """A branch literally named 'detached' should not be treated as + detached HEAD.""" + output = "## detached\n" + status = parse_git_status(output) + assert status.current_branch == "detached" + assert status.detached is False + + +def test_actual_detached_head(): + """Real detached HEAD should still be detected.""" + output = "## HEAD (detached at abc1234)\n" + status = parse_git_status(output) + assert status.detached is True + + +def test_branch_name_containing_detached_substring(): + """Branch names like 'fix-detached-bug' should not be treated as detached.""" + output = "## fix-detached-bug...origin/fix-detached-bug\n" + status = parse_git_status(output) + assert status.current_branch == "fix-detached-bug" + assert status.upstream == "origin/fix-detached-bug" + assert status.detached is False + + +def test_no_commits_yet(): + """'No commits yet on main' should parse as branch 'main', not detached.""" + output = "## No commits yet on main\n" + status = parse_git_status(output) + assert status.current_branch == "main" + assert status.detached is False + + +def test_head_no_branch(): + """'HEAD (no branch)' should be treated as detached.""" + output = "## HEAD (no branch)\n" + status = parse_git_status(output) + assert status.detached is True + + +def test_simple_branch_no_upstream(): + """A branch with no upstream tracking should parse correctly.""" + output = "## feature-branch\n" + status = parse_git_status(output) + assert status.current_branch == "feature-branch" + assert status.upstream is None + assert status.detached is False + + +def test_empty_output(): + """Empty output should return a clean status.""" + status = parse_git_status("") + assert status.current_branch is None + assert status.detached is False + assert status.file_status == [] + + +def test_normal_branch_with_upstream(): + """Normal branch tracking upstream should parse correctly.""" + output = "## main...origin/main [ahead 1, behind 2]\n" + status = parse_git_status(output) + assert status.current_branch == "main" + assert status.upstream == "origin/main" + assert status.ahead == 1 + assert status.behind == 2 + assert status.detached is False