Skip to content

feat(sandbox): pin git identity to driver#53

Open
Mingholy wants to merge 1 commit into
mainfrom
feat/ft-7-sandbox-git-author
Open

feat(sandbox): pin git identity to driver#53
Mingholy wants to merge 1 commit into
mainfrom
feat/ft-7-sandbox-git-author

Conversation

@Mingholy

@Mingholy Mingholy commented Jun 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • Pin sandbox git author environment to the driver/user git identity.
  • Read the personal .gitconfig identity and inject it into sandbox process environment when available.
  • Add server-side coverage for author env derivation.

Test plan

  • cd web && bunx tsc -b
  • bun run build
  • git diff --check
  • Focused server tests for sandbox git author env were run during implementation.
  • Lint not run: root, web, and server package manifests do not define a lint script.

Screenshots

No before/after screenshot is attached because this is backend/container environment behavior with no user-facing UI state.

@Mingholy Mingholy left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: PR #53

Summary

This PR reads the sandbox driver's personal .gitconfig (from $LOOPAT_HOME/personal/<user>/.gitconfig), parses the [user] section for name and email, and injects GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL into the container environment at podman create time. This ensures git commits made by the AI inside the sandbox carry the driving user's identity instead of a generic or empty author.

Implementation Quality

Architecture fit: good. The injection point (buildContainerEnv) is correct — env vars baked at container create time are inherited by every podman exec (both SDK and PTY), so the identity is consistent across all sandbox processes. Placing it before the extraEnv loop (lines 419-421) means callers can still override if needed, preserving the "caller env wins" contract already tested.

Parser scope: appropriate. The hand-rolled INI parser only targets [user] name/email — no need for a full gitconfig library. It correctly handles:

  • Multi-section files (tracks inUserSection state)
  • Comments (#, ;)
  • Quoted values (single/double, stripped)
  • Subsections like [user "foo"] are correctly rejected by the === "user" check

All-or-nothing return: good. Requiring both name and email (returning null otherwise) prevents half-configured identity where git would still prompt or fall back.

Test coverage: solid. Tests cover:

  • Happy path with realistic multi-section .gitconfig
  • Missing file → null
  • Incomplete config (name without email) → null
  • Integration via buildContainerEnv — both present and absent cases

Code Quality

Clean, readable, well-placed. No style issues. The function is exported and tested independently, which is good practice.

Two minor observations (non-blocking):

  1. include/includeIf directives: The parser doesn't follow git's [include] path = ... or [includeIf ...] directives. This is fine for the use case — the personal .gitconfig is a loopat-managed file, not the user's system ~/.gitconfig, so conditional includes are unlikely. Worth a one-line comment if you want to make the limitation explicit.

  2. Staleness window: Because env is deliberately excluded from hashCreateArgs (line 546-551 comment — correct design to avoid the PTY exit-137 regression), changing .gitconfig after a container is created won't take effect until the container is recreated for another reason (image drift, vault change, etc.). This matches how all other create-time env vars behave, so it's consistent — just worth being aware of. A future enhancement could detect identity drift in the exec-time path if this becomes a pain point, but that's out of scope here.

Security

No concerns.

  • The file read is bounded to personalDir(opts.createdBy) — a controlled path under $LOOPAT_HOME/personal/<user>/. No path traversal possible; personalDir is computed from the authenticated createdBy field.
  • Values are passed via podman create --env K=V, not shell-interpolated. Even if a .gitconfig contained adversarial content in the name/email fields, it would only affect the user's own sandbox container. This is a self-injection scenario with no privilege escalation.
  • The personal dir is already bind-mounted into the container (both virtual and host-absolute paths), so reading .gitconfig from it at container-build time doesn't expand the attack surface.

Verdict

LGTM. Clean feature, correct integration point, good test coverage, no security issues. The two observations above are informational, not blocking.

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