fix(adapter-utils): carry the workspace origin remote into transported git workspaces (#10873)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When an agent runs on a different host (sandbox or SSH), the adapter transport copies the local git execution workspace to that host and syncs changes back after the run > - The transport materializes the remote copy with `git init` plus a depth-1 or bundle fetch, so the copy has no `origin` remote and its head reads as a parentless snapshot commit > - An agent asked to publish its branch (push it, open a pull request) sees "no remote, root snapshot" and must hand the publish step back to a human operator, even when the branch base is a commit the upstream remote already holds > - This pull request carries the workspace's `origin` URL (credential-scrubbed) onto the transported copy as metadata > - The benefit is that branches produced in transported workspaces stay publishable by any actor with credentials, while the transport itself still never fetches or pushes ## Linked Issues or Issue Description No public issue exists. Description follows the enhancement template: **What existing behavior does this improve?** The workspace transport in `@paperclipai/adapter-utils` already copies a git workspace to the execution host and back. This change improves the fidelity of that copy: the transported repo keeps the workspace's `origin` remote instead of losing it. **Subsystem affected** Adapter utilities — the sandbox transport (`withShallowGitWorkspaceClone` in `packages/adapter-utils/src/git-workspace-sync.ts`) and the SSH transport (`importGitWorkspaceToSsh` in `packages/adapter-utils/src/ssh.ts`). **Current behavior** The transported copy is built with `git init` plus a depth-1 (sandbox) or bundle (SSH) fetch. It has no remotes. `git remote -v` is empty and the head commit reads as a root snapshot with no visible ancestry. Agents and operators inside the execution host cannot fetch real ancestry or push a branch, even when the branch base is a commit the upstream remote already holds. **Proposed behavior** The transport reads the source workspace's `origin` URL, scrubs credentials from it, and configures it on the transported copy. The sandbox path adds the remote to the fresh clone. The SSH path sets or adds the remote in the remote setup script, which also covers reused workspace directories. A workspace with no `origin` transports exactly as before. **Reason and benefit** A branch committed in a transported workspace becomes publishable in place: the shallow boundary commit already exists on the remote, so a push pack closes without full local ancestry (a new test locks in this property). Fetching real ancestry also becomes possible for whoever holds credentials. Without this, agents must describe their change in a handoff document and a human must reconstruct the branch by hand. **Breaking changes** None. The URL copy is best-effort and metadata-only. The transport never fetches from or pushes to the remote. The no-remote-git contract holds: sync-back through the local cwd stays the only cross-run persistence path, and `packages/adapters/AUTHORING.md` gains a paragraph that makes the carried-remote nuance explicit. ## What Changed - `packages/adapter-utils/src/git-workspace-sync.ts`: new `sanitizeGitRemoteUrl` (strips http(s) userinfo, where tokens can be embedded; scp-like/ssh forms and filesystem paths pass through) and `readSanitizedOriginRemoteUrl`; `withShallowGitWorkspaceClone` configures the scrubbed `origin` on the fresh clone, best-effort. - `packages/adapter-utils/src/ssh.ts`: `importGitWorkspaceToSsh` sets or adds the scrubbed `origin` in the remote setup script, non-fatal under `set -e`. - `packages/adapter-utils/src/git-workspace-sync.test.ts`: four new integration cases (remote copied, credentials scrubbed, no-origin unchanged, push from the shallow clone to an origin that holds the base commit) plus `sanitizeGitRemoteUrl` unit tests. - `packages/adapters/AUTHORING.md`: documents that a transported copy may carry a credential-scrubbed `origin` as metadata, and why this does not weaken the no-remote-git contract. ## Verification - `npx vitest run packages/adapter-utils/src/git-workspace-sync.test.ts` — 12/12 pass (4 new integration cases + sanitizer unit tests). - `npx vitest run packages/adapter-utils/src/sandbox-managed-runtime.test.ts` — 24/24 pass. - `npx vitest run packages/adapter-utils/src/ssh-fixture.test.ts` — 16/16 pass, including the `no-remote-git contract` case (a workspace without `origin` still round-trips with no remote introduced at any point). - `node scripts/check-no-git-push.mjs` — passes; this change adds no push or fetch to adapter/runtime code. - `pnpm typecheck` in `packages/adapter-utils` — clean. ## Risks - Low risk. The change is additive metadata on the transported copy only; failure to record the remote never fails the transport. - Credential exposure is the real hazard and is handled: http(s) userinfo is stripped before the URL leaves the host. Non-http forms (scp-like, `ssh://`) carry no secret in the URL and pass through. - A reused SSH workspace whose project `origin` changed now gets the current URL via `set-url` instead of keeping a stale one. ## Model Used Claude Fable 5 (`claude-fable-5`), Anthropic — extended thinking, agentic tool use via Claude Code CLI. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
Devin Foley
GitHub
parent
1fa36be353
commit
ffd62a4cbb
|
|
@ -14,6 +14,7 @@ import {
|
|||
isMissingGitPrerequisiteError,
|
||||
readGitWorkspaceSnapshot,
|
||||
runLocalGit,
|
||||
sanitizeGitRemoteUrl,
|
||||
withShallowGitWorkspaceClone,
|
||||
} from "./git-workspace-sync.js";
|
||||
|
||||
|
|
@ -73,6 +74,101 @@ describe("git workspace sync", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("copies the workspace origin remote into the shallow clone", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-origin-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
const repo = await createRepo(rootDir);
|
||||
await git(repo, ["remote", "add", "origin", "https://github.com/example/repo.git"]);
|
||||
|
||||
const snapshot = await readGitWorkspaceSnapshot(repo);
|
||||
await withShallowGitWorkspaceClone({
|
||||
localDir: repo,
|
||||
snapshot: snapshot!,
|
||||
}, async (cloneDir) => {
|
||||
expect(await git(cloneDir, ["remote", "get-url", "origin"])).toBe("https://github.com/example/repo.git");
|
||||
});
|
||||
});
|
||||
|
||||
it("scrubs credentials from the origin remote before copying it", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-origin-scrub-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
const repo = await createRepo(rootDir);
|
||||
await git(repo, ["remote", "add", "origin", "https://x-access-token:sekret@github.com/example/repo.git"]);
|
||||
|
||||
const snapshot = await readGitWorkspaceSnapshot(repo);
|
||||
await withShallowGitWorkspaceClone({
|
||||
localDir: repo,
|
||||
snapshot: snapshot!,
|
||||
}, async (cloneDir) => {
|
||||
expect(await git(cloneDir, ["remote", "get-url", "origin"])).toBe("https://github.com/example/repo.git");
|
||||
});
|
||||
});
|
||||
|
||||
it("leaves the shallow clone remote-less when the workspace has no origin", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-no-origin-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
const repo = await createRepo(rootDir);
|
||||
|
||||
const snapshot = await readGitWorkspaceSnapshot(repo);
|
||||
await withShallowGitWorkspaceClone({
|
||||
localDir: repo,
|
||||
snapshot: snapshot!,
|
||||
}, async (cloneDir) => {
|
||||
await expect(git(cloneDir, ["remote", "get-url", "origin"])).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
it("drops a filesystem-path origin instead of copying it into the shallow clone", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-path-origin-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
const repo = await createRepo(rootDir);
|
||||
await git(repo, ["remote", "add", "origin", path.join(rootDir, "elsewhere.git")]);
|
||||
|
||||
const snapshot = await readGitWorkspaceSnapshot(repo);
|
||||
await withShallowGitWorkspaceClone({
|
||||
localDir: repo,
|
||||
snapshot: snapshot!,
|
||||
}, async (cloneDir) => {
|
||||
await expect(git(cloneDir, ["remote", "get-url", "origin"])).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
it("pushes new commits from the shallow clone to an origin that holds the base commit", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-shallow-push-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
const repo = await createRepo(rootDir);
|
||||
const upstream = path.join(rootDir, "upstream.git");
|
||||
await mkdir(upstream, { recursive: true });
|
||||
await git(upstream, ["init", "--bare"]);
|
||||
await git(repo, ["remote", "add", "origin", upstream]);
|
||||
await git(repo, ["push", "origin", "main"]);
|
||||
const baseHead = await git(repo, ["rev-parse", "HEAD"]);
|
||||
|
||||
const snapshot = await readGitWorkspaceSnapshot(repo);
|
||||
await withShallowGitWorkspaceClone({
|
||||
localDir: repo,
|
||||
snapshot: snapshot!,
|
||||
}, async (cloneDir) => {
|
||||
await git(cloneDir, ["config", "user.name", "Paperclip Sandbox"]);
|
||||
await git(cloneDir, ["config", "user.email", "sandbox@paperclip.dev"]);
|
||||
await writeFile(path.join(cloneDir, "change.txt"), "sandbox change\n", "utf8");
|
||||
await git(cloneDir, ["add", "change.txt"]);
|
||||
await git(cloneDir, ["commit", "-m", "sandbox change"]);
|
||||
const cloneHead = await git(cloneDir, ["rev-parse", "HEAD"]);
|
||||
|
||||
// A filesystem-path origin is dropped by the allowlist, so configure the
|
||||
// remote explicitly — the property under test is the push itself: the
|
||||
// clone is shallow (single grafted commit), but the boundary commit
|
||||
// already exists on the origin, so the push pack closes without full
|
||||
// ancestry. That is what makes transported branches publishable.
|
||||
await git(cloneDir, ["remote", "add", "origin", upstream]);
|
||||
await git(cloneDir, ["push", "origin", "HEAD:refs/heads/sandbox-change"]);
|
||||
|
||||
expect(await git(upstream, ["rev-parse", "refs/heads/sandbox-change"])).toBe(cloneHead);
|
||||
expect(await git(upstream, ["merge-base", "refs/heads/main", "refs/heads/sandbox-change"])).toBe(baseHead);
|
||||
});
|
||||
});
|
||||
|
||||
it("builds thin git delta bundles relative to the imported base", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-delta-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
|
|
@ -300,3 +396,47 @@ describe("git workspace sync", () => {
|
|||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("sanitizeGitRemoteUrl", () => {
|
||||
it("strips userinfo, query, and fragment from http(s) URLs", () => {
|
||||
expect(sanitizeGitRemoteUrl("https://x-access-token:sekret@github.com/example/repo.git"))
|
||||
.toBe("https://github.com/example/repo.git");
|
||||
expect(sanitizeGitRemoteUrl("https://sekret-token@github.com/example/repo.git"))
|
||||
.toBe("https://github.com/example/repo.git");
|
||||
expect(sanitizeGitRemoteUrl("http://user:pass@git.internal/example/repo.git"))
|
||||
.toBe("http://git.internal/example/repo.git");
|
||||
expect(sanitizeGitRemoteUrl("https://github.com/example/repo.git?private_token=sekret#fragment"))
|
||||
.toBe("https://github.com/example/repo.git");
|
||||
});
|
||||
|
||||
it("strips password and query from ssh-scheme URLs but keeps the username", () => {
|
||||
expect(sanitizeGitRemoteUrl("ssh://git@github.com/example/repo.git"))
|
||||
.toBe("ssh://git@github.com/example/repo.git");
|
||||
expect(sanitizeGitRemoteUrl("ssh://git:sekret@github.com/example/repo.git"))
|
||||
.toBe("ssh://git@github.com/example/repo.git");
|
||||
expect(sanitizeGitRemoteUrl("git+ssh://git@github.com/example/repo.git?key=sekret"))
|
||||
.toBe("git+ssh://git@github.com/example/repo.git");
|
||||
});
|
||||
|
||||
it("keeps credential-free scp-like remotes unchanged", () => {
|
||||
expect(sanitizeGitRemoteUrl("git@github.com:example/repo.git"))
|
||||
.toBe("git@github.com:example/repo.git");
|
||||
expect(sanitizeGitRemoteUrl("https://github.com/example/repo.git"))
|
||||
.toBe("https://github.com/example/repo.git");
|
||||
});
|
||||
|
||||
it("drops every shape whose credential surface is unknown", () => {
|
||||
// Filesystem paths are useless on the execution host and could leak
|
||||
// host-layout details; unknown schemes and malformed userinfo could carry
|
||||
// embedded secrets the sanitizer cannot recognize. All fail closed.
|
||||
expect(sanitizeGitRemoteUrl("/tmp/local/upstream.git")).toBeNull();
|
||||
expect(sanitizeGitRemoteUrl("ftp://user:pass@host/repo.git")).toBeNull();
|
||||
expect(sanitizeGitRemoteUrl("user:pass@host:path/repo.git")).toBeNull();
|
||||
expect(sanitizeGitRemoteUrl("host.example:path/repo.git")).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null for empty input", () => {
|
||||
expect(sanitizeGitRemoteUrl("")).toBeNull();
|
||||
expect(sanitizeGitRemoteUrl(" ")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -110,6 +110,67 @@ export async function readGitWorkspaceSnapshot(localDir: string): Promise<GitWor
|
|||
}
|
||||
}
|
||||
|
||||
// scp-like ssh remote (`user@host:path`). The syntax has no password slot, so
|
||||
// it cannot embed a secret. Conservative shape: exactly one `@`, no colon in
|
||||
// the user segment (a colon there could smuggle credential-looking material),
|
||||
// no scheme separator (a `://` form parses as a URL and never reaches this).
|
||||
const SCP_LIKE_REMOTE_PATTERN = /^[^@:/\s]+@[^@:/\s]+:\S+$/;
|
||||
|
||||
/**
|
||||
* Reduce a git remote URL to a credential-free form before it is copied into a
|
||||
* transported workspace, or null when the URL must not be carried at all.
|
||||
* Allowlist, fail closed: only shapes whose credential surface is fully known
|
||||
* are kept — http(s) with userinfo/query/fragment stripped (tokens ride in any
|
||||
* of those), ssh/git schemes with password/query/fragment stripped, and
|
||||
* scp-like `user@host:path` (no password slot exists in that syntax). Every
|
||||
* other form — filesystem paths, unknown schemes, unparseable strings — is
|
||||
* dropped rather than risk persisting an embedded secret in the execution
|
||||
* host's git config.
|
||||
*/
|
||||
export function sanitizeGitRemoteUrl(url: string): string | null {
|
||||
const trimmed = url.trim();
|
||||
if (!trimmed) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
const parsed = new URL(trimmed);
|
||||
if (parsed.protocol === "http:" || parsed.protocol === "https:") {
|
||||
parsed.username = "";
|
||||
parsed.password = "";
|
||||
parsed.search = "";
|
||||
parsed.hash = "";
|
||||
return parsed.toString();
|
||||
}
|
||||
if (parsed.protocol === "ssh:" || parsed.protocol === "git:" || parsed.protocol === "git+ssh:") {
|
||||
// The username (conventionally `git`) is addressing, not a secret; a
|
||||
// password or query string can be, so those are stripped.
|
||||
parsed.password = "";
|
||||
parsed.search = "";
|
||||
parsed.hash = "";
|
||||
return parsed.toString();
|
||||
}
|
||||
return null;
|
||||
} catch {
|
||||
return SCP_LIKE_REMOTE_PATTERN.test(trimmed) ? trimmed : null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The workspace's `origin` remote URL with credentials scrubbed, or null when
|
||||
* the workspace has no `origin` remote (or is not a git repository).
|
||||
*/
|
||||
export async function readSanitizedOriginRemoteUrl(localDir: string): Promise<string | null> {
|
||||
try {
|
||||
const result = await runLocalGit(localDir, ["remote", "get-url", "origin"], {
|
||||
timeout: 10_000,
|
||||
maxBuffer: 16 * 1024,
|
||||
});
|
||||
return sanitizeGitRemoteUrl(result.stdout.trim());
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export async function withShallowGitWorkspaceClone<T>(
|
||||
input: {
|
||||
localDir: string;
|
||||
|
|
@ -120,6 +181,7 @@ export async function withShallowGitWorkspaceClone<T>(
|
|||
const cloneDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-git-workspace-"));
|
||||
const tempRef = `refs/paperclip/git-sync/import/${randomUUID()}`;
|
||||
try {
|
||||
const originUrl = await readSanitizedOriginRemoteUrl(input.localDir);
|
||||
await runLocalGit(input.localDir, ["update-ref", tempRef, input.snapshot.headCommit], {
|
||||
timeout: 10_000,
|
||||
maxBuffer: 16 * 1024,
|
||||
|
|
@ -128,6 +190,19 @@ export async function withShallowGitWorkspaceClone<T>(
|
|||
timeout: 10_000,
|
||||
maxBuffer: 64 * 1024,
|
||||
});
|
||||
if (originUrl) {
|
||||
// The clone is what lands in the sandbox. Without `origin`, the branch
|
||||
// there reads as an unpublishable root snapshot even though its head is a
|
||||
// commit the upstream remote already holds — so fetch (to reconnect
|
||||
// ancestry) and push (to publish the branch; the shallow boundary commit
|
||||
// is already on the remote, so the pack closes) are both mechanically
|
||||
// possible once the remote is carried over. Best-effort: a failure to
|
||||
// record the remote must not fail the transport.
|
||||
await runLocalGit(cloneDir, ["remote", "add", "origin", originUrl], {
|
||||
timeout: 10_000,
|
||||
maxBuffer: 16 * 1024,
|
||||
}).catch(() => undefined);
|
||||
}
|
||||
await runLocalGit(cloneDir, ["fetch", "--depth=1", input.localDir, tempRef], {
|
||||
timeout: 60_000,
|
||||
maxBuffer: 1024 * 1024,
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import os from "node:os";
|
|||
import path from "node:path";
|
||||
import { Transform } from "node:stream";
|
||||
import type { CommandManagedRuntimeRunner } from "./command-managed-runtime.js";
|
||||
import { readSanitizedOriginRemoteUrl } from "./git-workspace-sync.js";
|
||||
import type { RunProcessResult } from "./server-utils.js";
|
||||
import type { DirectorySnapshot } from "./workspace-restore-merge.js";
|
||||
import { mergeDirectoryWithBaseline } from "./workspace-restore-merge.js";
|
||||
|
|
@ -776,6 +777,7 @@ async function importGitWorkspaceToSsh(input: {
|
|||
timeout: 60_000,
|
||||
maxBuffer: 1024 * 1024,
|
||||
});
|
||||
const originUrl = await readSanitizedOriginRemoteUrl(input.localDir);
|
||||
|
||||
const remoteSetupScript = [
|
||||
"set -e",
|
||||
|
|
@ -784,6 +786,15 @@ async function importGitWorkspaceToSsh(input: {
|
|||
'trap \'rm -f "$tmp_bundle"\' EXIT',
|
||||
'cat > "$tmp_bundle"',
|
||||
`if [ ! -d ${shellQuote(path.posix.join(input.remoteDir, ".git"))} ]; then git init ${shellQuote(input.remoteDir)} >/dev/null; fi`,
|
||||
// Carry the workspace's (credential-scrubbed) origin into the transported
|
||||
// repo so branches there keep a publishable remote instead of reading as
|
||||
// remote-less snapshots. set-url covers a reused workspace whose origin
|
||||
// changed; add covers the fresh-init case. Best-effort under `set -e`.
|
||||
...(originUrl
|
||||
? [
|
||||
`{ git -C ${shellQuote(input.remoteDir)} remote set-url origin ${shellQuote(originUrl)} >/dev/null 2>&1 || git -C ${shellQuote(input.remoteDir)} remote add origin ${shellQuote(originUrl)} >/dev/null 2>&1; } || true`,
|
||||
]
|
||||
: []),
|
||||
`git -C ${shellQuote(input.remoteDir)} fetch --force "$tmp_bundle" '${tempRef}:${tempRef}' >/dev/null`,
|
||||
input.snapshot.branchName
|
||||
? `git -C ${shellQuote(input.remoteDir)} checkout --force -B ${shellQuote(input.snapshot.branchName)} ${shellQuote(input.snapshot.headCommit)} >/dev/null`
|
||||
|
|
|
|||
|
|
@ -38,6 +38,18 @@ How to apply:
|
|||
`workspace_finalize=failed` on the execution workspace, which gates
|
||||
dependent issue wakes until the next successful finalize. Do not swallow
|
||||
restore errors.
|
||||
- A transported workspace copy *may* carry the local workspace's `origin`
|
||||
remote URL so that branches in the copy stay publishable by the agent or an
|
||||
operator who holds credentials — the transport helpers copy the URL as
|
||||
metadata only. The copy is allowlist-based and fails closed
|
||||
(`sanitizeGitRemoteUrl`): http(s) URLs are stripped of userinfo, query, and
|
||||
fragment; `ssh:`/`git:` scheme URLs are stripped of password and query;
|
||||
scp-like `user@host:path` passes through (the syntax has no password slot);
|
||||
every other shape — filesystem paths, unknown schemes — is dropped rather
|
||||
than risk persisting an embedded secret. This does not weaken the contract:
|
||||
sync-back through the local cwd remains the only cross-run persistence
|
||||
path, the helpers never fetch from or push to that remote, and a workspace
|
||||
without an `origin` transports exactly as before.
|
||||
|
||||
The invariant is pinned by the `no-remote-git contract` case in
|
||||
[`packages/adapter-utils/src/ssh-fixture.test.ts`](../adapter-utils/src/ssh-fixture.test.ts),
|
||||
|
|
|
|||
Loading…
Reference in New Issue