feat: git-backed pipeline audit trail#19
Conversation
Implements the core git-sync service that handles cloning, committing, and pushing pipeline YAML files to a configured Git repository. Supports both commit (deploy) and delete operations with temp directory cleanup and non-throwing error handling.
Call gitSyncCommitPipeline after version creation in deployAgent. The sync runs as a non-blocking side effect — if it fails, the deploy still succeeds and the error is surfaced via gitSyncError in the result.
- Strip encrypted gitToken from environment.get API response (defense-in-depth) - Clone into subdirectory of temp dir to avoid non-empty directory errors - Add HTTPS-only validation for git repo URLs (SSRF protection) - Add branch name regex validation - Sanitize git author name/email to prevent malformed author strings - Handle file-not-found gracefully in gitSyncDeletePipeline
Greptile SummaryThis PR adds a git-backed audit trail for pipeline deployments — on each deploy or delete, VectorFlow commits the generated pipeline YAML to a configured HTTPS Git repository, with the PAT encrypted at rest using the existing AES-256-GCM pattern. Several issues from earlier review rounds have been properly addressed: Two issues remain that should be addressed before merging:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User (Browser)
participant R as environment.ts router
participant DA as deploy-agent.ts
participant GS as git-sync.ts
participant DB as PostgreSQL
participant GR as Git Remote
U->>R: testGitConnection(repoUrl, branch, token)
R->>R: HTTPS check, withTeamAccess("EDITOR")
R->>GR: git clone --depth 1 (token in URL)
GR-->>R: success / error (sanitized)
R-->>U: { success, error? }
U->>DA: deploy.agent(pipelineId)
DA->>DA: createVersion, push to nodes
DA->>DB: findUnique(environment)
DB-->>DA: { gitRepoUrl, gitToken (encrypted) }
DA->>GS: gitSyncCommitPipeline(config, yaml)
GS->>GS: decrypt(encryptedToken)
GS->>GR: git clone --depth 1
GS->>GR: git add, commit, push
GR-->>GS: success / non-fast-forward error
GS-->>DA: { success, error? }
DA-->>U: { versionId, gitSyncError? }
U->>R: pipeline.delete(id)
R->>GS: gitSyncDeletePipeline (awaited)
GS->>GR: git clone --depth 1, rm, commit, push
GR-->>GS: result
GS-->>R: (error swallowed via .catch)
R->>DB: prisma.pipeline.delete
DB-->>R: done
R-->>U: deleted pipeline
Last reviewed commit: e38c3f6 |
- Sanitize git error messages to strip credentials from URLs - Add withAudit middleware to testGitConnection mutation - Add environmentId to testGitConnection for proper team scoping
| .mutation(async ({ input }) => { | ||
| const parsedUrl = new URL(input.repoUrl); | ||
| if (parsedUrl.protocol !== "https:") { | ||
| return { success: false, error: "Only HTTPS repository URLs are supported" }; | ||
| } | ||
|
|
||
| const simpleGit = (await import("simple-git")).default; | ||
| const { mkdtemp, rm } = await import("fs/promises"); | ||
| const { join } = await import("path"); | ||
| const { tmpdir } = await import("os"); | ||
|
|
||
| let workdir: string | null = null; | ||
| try { | ||
| workdir = await mkdtemp(join(tmpdir(), "vf-git-test-")); | ||
| const repoDir = join(workdir, "repo"); | ||
| const git = simpleGit(workdir); | ||
| parsedUrl.username = input.token; | ||
| parsedUrl.password = ""; | ||
| await git.clone(parsedUrl.toString(), repoDir, [ | ||
| "--branch", input.branch, | ||
| "--depth", "1", | ||
| "--single-branch", | ||
| ]); | ||
| return { success: true }; |
There was a problem hiding this comment.
SSRF via user-supplied repo URL
testGitConnection initiates an outbound git clone to any HTTPS URL supplied by an authenticated EDITOR. The HTTPS-only check guards against the most common HTTP-based cloud metadata endpoints (e.g., AWS at http://169.254.169.254/latest/meta-data/), but internal services that are HTTPS-accessible — such as a private Kubernetes API server, internal Nexus/Artifactory, or a corporate identity provider — can still be reached.
An EDITOR in one team can use this endpoint to probe whether internal services are reachable by observing error responses and timing differences, without ever having access to those services themselves.
Consider adding a private IP / loopback blocklist before the clone attempt:
const BLOCKED_HOSTS = /^(localhost|127\.|10\.|172\.(1[6-9]|2\d|3[01])\.|192\.168\.|169\.254\.|::1|fc00:|fd)/i;
const hostname = parsedUrl.hostname;
if (BLOCKED_HOSTS.test(hostname)) {
return { success: false, error: "Private and loopback addresses are not allowed" };
}The same guard should be applied inside gitSyncCommitPipeline and gitSyncDeletePipeline in git-sync.ts, since a saved gitRepoUrl set via the update mutation is also passed to those functions with the stored (decrypted) PAT.
Context Used: Rule from dashboard - ## Security & Cryptography Review Rules
When reviewing changes to authentication, authorization, en... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/server/routers/environment.ts
Line: 152-175
Comment:
**SSRF via user-supplied repo URL**
`testGitConnection` initiates an outbound `git clone` to any HTTPS URL supplied by an authenticated EDITOR. The HTTPS-only check guards against the most common HTTP-based cloud metadata endpoints (e.g., AWS at `http://169.254.169.254/latest/meta-data/`), but internal services that are HTTPS-accessible — such as a private Kubernetes API server, internal Nexus/Artifactory, or a corporate identity provider — can still be reached.
An EDITOR in one team can use this endpoint to probe whether internal services are reachable by observing error responses and timing differences, without ever having access to those services themselves.
Consider adding a private IP / loopback blocklist before the clone attempt:
```typescript
const BLOCKED_HOSTS = /^(localhost|127\.|10\.|172\.(1[6-9]|2\d|3[01])\.|192\.168\.|169\.254\.|::1|fc00:|fd)/i;
const hostname = parsedUrl.hostname;
if (BLOCKED_HOSTS.test(hostname)) {
return { success: false, error: "Private and loopback addresses are not allowed" };
}
```
The same guard should be applied inside `gitSyncCommitPipeline` and `gitSyncDeletePipeline` in `git-sync.ts`, since a saved `gitRepoUrl` set via the `update` mutation is also passed to those functions with the stored (decrypted) PAT.
**Context Used:** Rule from `dashboard` - ## Security & Cryptography Review Rules
When reviewing changes to authentication, authorization, en... ([source](https://app.greptile.com/review/custom-context?memory=7cb20c56-ca6a-40aa-8660-7fa75e6e3db2))
How can I resolve this? If you propose a fix, please make it concise.| const git: SimpleGit = simpleGit(workdir); | ||
| await git.clone(url, repoDir, ["--branch", config.branch, "--depth", "1", "--single-branch"]); | ||
| const repoGit: SimpleGit = simpleGit(repoDir); | ||
|
|
||
| // Write the pipeline YAML file | ||
| const envDir = toFilenameSlug(environmentName); | ||
| const filename = `${toFilenameSlug(pipelineName)}.yaml`; | ||
| const filePath = join(envDir, filename); | ||
| const fullPath = join(repoDir, filePath); | ||
|
|
||
| await mkdir(join(repoDir, envDir), { recursive: true }); | ||
| await writeFile(fullPath, configYaml, "utf-8"); | ||
|
|
||
| await repoGit.add(filePath); | ||
|
|
||
| // Check if there are actually changes to commit | ||
| const status = await repoGit.status(); | ||
| if (status.isClean()) { | ||
| return { success: true, commitSha: "no-change" }; | ||
| } | ||
|
|
||
| await repoGit.addConfig("user.name", author.name || "VectorFlow User"); | ||
| await repoGit.addConfig("user.email", author.email || "noreply@vectorflow"); | ||
| await repoGit.commit(commitMessage, filePath, { | ||
| "--author": sanitizeAuthor(author.name, author.email), | ||
| }); | ||
| await repoGit.push("origin", config.branch); |
There was a problem hiding this comment.
Shallow-clone push fails on concurrent deploys
Both gitSyncCommitPipeline and gitSyncDeletePipeline use --depth 1 clones. If two pipelines in the same environment are deployed concurrently (or a deploy and a delete overlap), the sequence becomes:
- Deploy A clones branch at commit X
- Deploy B clones branch at commit X
- Deploy A pushes commit Y — succeeds
- Deploy B tries to push commit Y' — rejected: non-fast-forward (remote is now at Y)
Because the local repo has only one commit of history (--depth 1), there is no way to run git pull --rebase to recover. The push fails, and the user sees a git sync warning toast even though the deploy itself succeeded. In environments with pipelines frequently deployed together (e.g., automated CI deploys), this failure mode will be common.
Options:
- Remove
--depth 1to allowgit pull --rebasebefore pushing (adds latency) - Retry the entire clone→commit→push cycle on non-fast-forward failures (up to N times)
- Document this limitation explicitly in the user guide
The same issue applies to gitSyncDeletePipeline (line 137).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/server/services/git-sync.ts
Line: 77-103
Comment:
**Shallow-clone push fails on concurrent deploys**
Both `gitSyncCommitPipeline` and `gitSyncDeletePipeline` use `--depth 1` clones. If two pipelines in the same environment are deployed concurrently (or a deploy and a delete overlap), the sequence becomes:
1. Deploy A clones branch at commit **X**
2. Deploy B clones branch at commit **X**
3. Deploy A pushes commit **Y** — succeeds
4. Deploy B tries to push commit **Y'** — **rejected: non-fast-forward** (remote is now at Y)
Because the local repo has only one commit of history (`--depth 1`), there is no way to run `git pull --rebase` to recover. The push fails, and the user sees a git sync warning toast even though the deploy itself succeeded. In environments with pipelines frequently deployed together (e.g., automated CI deploys), this failure mode will be common.
Options:
- Remove `--depth 1` to allow `git pull --rebase` before pushing (adds latency)
- Retry the entire clone→commit→push cycle on non-fast-forward failures (up to N times)
- Document this limitation explicitly in the user guide
The same issue applies to `gitSyncDeletePipeline` (line 137).
How can I resolve this? If you propose a fix, please make it concise.| const data: Record<string, unknown> = { ...rest }; | ||
| if (gitToken !== undefined) { | ||
| data.gitToken = gitToken ? encrypt(gitToken) : null; | ||
| } | ||
|
|
||
| const updated = await prisma.environment.update({ | ||
| where: { id }, | ||
| data, |
There was a problem hiding this comment.
Record<string, unknown> bypasses Prisma type safety
Using Record<string, unknown> as the type for data loses all compile-time checking for the Prisma update. Field name typos and type mismatches won't be caught by tsc, and unknown fields would be silently ignored by Prisma at runtime.
Consider using the Prisma-generated input type (Prisma.EnvironmentUpdateInput) and constructing the object with explicit field assignments. This would restore TypeScript's ability to catch invalid field names and incorrect value types before runtime.
Context Used: Rule from dashboard - ## Code Style & Conventions
TypeScript Conventions
- Strict mode enabled, avoid
any— use `un... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/server/routers/environment.ts
Line: 126-133
Comment:
**`Record<string, unknown>` bypasses Prisma type safety**
Using `Record<string, unknown>` as the type for `data` loses all compile-time checking for the Prisma update. Field name typos and type mismatches won't be caught by `tsc`, and unknown fields would be silently ignored by Prisma at runtime.
Consider using the Prisma-generated input type (`Prisma.EnvironmentUpdateInput`) and constructing the object with explicit field assignments. This would restore TypeScript's ability to catch invalid field names and incorrect value types before runtime.
**Context Used:** Rule from `dashboard` - ## Code Style & Conventions
### TypeScript Conventions
- Strict mode enabled, avoid `any` — use `un... ([source](https://app.greptile.com/review/custom-context?memory=6ae51394-d0b6-4686-bc4c-1ad840c2e310))
How can I resolve this? If you propose a fix, please make it concise.
Summary
git-syncservice handles clone/commit/push in temp directories with non-blocking error handling (failures never gate deploys)Changes
prisma/schema.prisma— 3 nullable fields on Environmentsrc/server/services/git-sync.ts— commit/delete pipeline YAMLsrc/server/services/deploy-agent.ts— post-deploy git syncsrc/server/routers/pipeline.ts— pre-delete git syncsrc/server/routers/environment.ts— git config CRUD + test connectionsrc/components/environment/git-sync-section.tsx— settings cardsrc/components/flow/deploy-dialog.tsx— git sync warning toastdocs/public/user-guide/environments.md,docs/public/operations/security.mdTest plan
{env-name}/{pipeline-name}.yaml