From e6d4f44a6122dab47fbee2a2a2b4330a62841bed Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 11 Sep 2026 09:25:24 +0200 Subject: [PATCH] fix: show symlinked files in the git repo viewer (#11081) * docs: describe symlink handling in the repo viewer's hub script Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FPgYLu8Eiq58Bv2ev2YRYN * docs: describe the symlink budget in the repo viewer's hub script Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FPgYLu8Eiq58Bv2ev2YRYN * docs: link the repo viewer script's hub page and soften the skip-log claim Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FPgYLu8Eiq58Bv2ev2YRYN * docs: charge the symlink budget before a link is resolved Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FPgYLu8Eiq58Bv2ev2YRYN * fix: point the git repo viewer at the symlink-following clone script Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FPgYLu8Eiq58Bv2ev2YRYN * docs: note that a new pin doesn't refresh commits already uploaded Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FPgYLu8Eiq58Bv2ev2YRYN --------- Co-authored-by: Claude Opus 5 (1M context) --- ...repo_and_upload_to_instance_storage.bun.ts | 390 ------------------ docs/git-repo-viewer-hub-script.md | 113 ++++- frontend/src/lib/hubPaths.json | 2 +- 3 files changed, 103 insertions(+), 402 deletions(-) delete mode 100644 docs/clone_repo_and_upload_to_instance_storage.bun.ts diff --git a/docs/clone_repo_and_upload_to_instance_storage.bun.ts b/docs/clone_repo_and_upload_to_instance_storage.bun.ts deleted file mode 100644 index 485ac0c766..0000000000 --- a/docs/clone_repo_and_upload_to_instance_storage.bun.ts +++ /dev/null @@ -1,390 +0,0 @@ -import * as wmillclient from "windmill-client"; -import { basename, join } from "node:path"; -import { existsSync, rmSync } from "fs"; -import process from "process"; -import { spawn } from 'child_process'; -import * as fs_async from 'fs/promises'; -import * as fs from 'node:fs'; - -const UPLOAD_CONCURRENCY = 16; -const CLONE_MARKER_FILE = ".windmill_clone_complete"; - -type GitRepository = { - url: string; - branch: string; - folder: string; - gpg_key: any; - is_github_app: boolean; -}; - -export async function main( - resource_path: string, - workspace: string, - git_ssh_identity?: string[], - commit?: string -) { - let clonedRepoPath: string | undefined; - - try { - console.log("Starting git clone and Blob storage upload process"); - - // Get the git repository resource - const repo_resource: GitRepository = await wmillclient.getResource(resource_path); - - const cwd = process.cwd(); - - if (git_ssh_identity) { - process.env.GIT_SSH_COMMAND = await get_git_ssh_cmd(cwd, git_ssh_identity) - } - - // Handle GitHub App authentication if needed - if (repo_resource.is_github_app) { - const token = await get_gh_app_token(); - repo_resource.url = prependTokenToGitHubUrl(repo_resource.url, token); - } - - process.env["HOME"] = "."; - process.env.GIT_TERMINAL_PROMPT = "0"; - - // Clone the repository - const { repo_name, commitHash } = await git_clone(cwd, repo_resource, commit); - clonedRepoPath = join(cwd, repo_name); - - // Remove .git directory to avoid uploading git history - const gitDir = join(clonedRepoPath, ".git"); - if (existsSync(gitDir)) { - rmSync(gitDir, { recursive: true, force: true }); - console.log("Removed .git directory"); - } - - // Upload to S3 - const s3Path = `gitrepos/${workspace}/${resource_path}/${commitHash}`; - const fileCount = await uploadDirectoryToS3(clonedRepoPath, s3Path, workspace); - - return { - success: true, - message: "Repository cloned and uploaded to S3 successfully", - s3_path: s3Path, - commit_hash: commitHash, - file_count: fileCount, - }; - - } catch (error) { - console.error("Error in git clone and upload:", error); - throw error; - } finally { - // Clean up cloned repository - if (clonedRepoPath && existsSync(clonedRepoPath)) { - rmSync(clonedRepoPath, { recursive: true, force: true }); - console.log("Cleaned up cloned repository"); - } - } -} - -async function get_git_ssh_cmd(cwd: string, git_ssh_identity: string[]): Promise { - const sshIdFiles = await Promise.all( - git_ssh_identity.map(async (varPath, i) => { - const filePath = join(cwd, `./ssh_id_priv_${i}`); - - try { - // Get variable value using windmill - let content = await wmillclient.getVariable(varPath); - content += '\n'; - - // Write file with content - await fs_async.writeFile(filePath, content, { encoding: 'utf8' }); - - // Set file permissions to 0o600 (read/write for owner only) - await fs_async.chmod(filePath, 0o600); - - // Escape single quotes for shell command - const escapedPath = filePath.replace(/'/g, "'\\''"); - return ` -i '${escapedPath}'`; - } catch (error) { - console.error( - `Variable ${varPath} not found for git ssh identity: ${error}` - ); - return ''; - } - }) - ); - - const gitSshCmd = `ssh -o StrictHostKeyChecking=no${sshIdFiles.join('')}`; - return gitSshCmd; -} - -async function git_clone( - cwd: string, - repo_resource: GitRepository, - commit?: string, -): Promise<{ repo_name: string; commitHash: string }> { - if (commit) { - return git_clone_at_commit(cwd, repo_resource, commit); - } else { - return git_clone_at_latest(cwd, repo_resource); - } -} - -async function git_clone_at_commit( - cwd: string, - repo_resource: GitRepository, - commit: string, -): Promise<{ repo_name: string; commitHash: string }> { - let repo_url = repo_resource.url; - const subfolder = repo_resource.folder ?? ""; - let branch = repo_resource.branch ?? ""; - const repo_name = basename(repo_url, ".git"); - - const azureMatch = repo_url.match(/AZURE_DEVOPS_TOKEN\((?.+)\)/); - if (azureMatch) { - console.log("Fetching Azure DevOps access token..."); - const azureResource = await wmillclient.getResource(azureMatch.groups.url); - const response = await fetch( - `https://login.microsoftonline.com/${azureResource.azureTenantId}/oauth2/token`, - { - method: "POST", - body: new URLSearchParams({ - client_id: azureResource.azureClientId, - client_secret: azureResource.azureClientSecret, - grant_type: "client_credentials", - resource: "499b84ac-1321-427f-aa17-267ca6975798/.default", - }), - } - ); - const { access_token } = await response.json(); - repo_url = repo_url.replace(azureMatch[0], access_token); - } - - const repoPath = join(cwd, repo_name); - await fs_async.mkdir(repoPath, { recursive: true }); - - process.chdir(repoPath); - - let args = ['init', '--quiet'] - if (branch) { - args.push(`--initial-branch=${branch}`) - } - await runCommand(undefined, 'git', ...args); - - await runCommand(0, 'git', 'remote', 'add', 'origin', repo_url); - - await runCommand(undefined, 'git', 'fetch', '--depth=1', '--quiet', 'origin', commit); - - await runCommand(undefined, 'git', 'checkout', '--quiet', 'FETCH_HEAD'); - - const commitHash = (await runCommand(undefined, "git", "rev-parse", "HEAD")).trim(); - - // Return to original directory - process.chdir(cwd); - - return { repo_name, commitHash }; -} - -async function git_clone_at_latest( - cwd: string, - repo_resource: GitRepository -): Promise<{ repo_name: string; commitHash: string }> { - let repo_url = repo_resource.url; - const subfolder = repo_resource.folder ?? ""; - let branch = repo_resource.branch ?? ""; - const repo_name = basename(repo_url, ".git"); - - // Handle Azure DevOps token if needed - const azureMatch = repo_url.match(/AZURE_DEVOPS_TOKEN\((?.+)\)/); - if (azureMatch) { - console.log("Fetching Azure DevOps access token..."); - const azureResource = await wmillclient.getResource(azureMatch.groups.url); - const response = await fetch( - `https://login.microsoftonline.com/${azureResource.azureTenantId}/oauth2/token`, - { - method: "POST", - body: new URLSearchParams({ - client_id: azureResource.azureClientId, - client_secret: azureResource.azureClientSecret, - grant_type: "client_credentials", - resource: "499b84ac-1321-427f-aa17-267ca6975798/.default", - }), - } - ); - const { access_token } = await response.json(); - repo_url = repo_url.replace(azureMatch[0], access_token); - } - - const args = ["clone", "--quiet", "--depth", "1"]; - if (subfolder !== "") args.push("--sparse"); - if (branch !== "") args.push("--branch", branch); - args.push(repo_url, repo_name); - - await runCommand(-1, "git", ...args); - - const fullPath = join(cwd, repo_name); - process.chdir(fullPath); - - if (subfolder !== "") { - await runCommand(undefined, "git", "sparse-checkout", "add", subfolder); - const subfolderPath = join(fullPath, subfolder); - - if (!existsSync(subfolderPath)) { - throw new Error(`Subfolder ${subfolder} does not exist.`); - } - - process.chdir(subfolderPath); - } - - // Get the commit hash - const commitHash = (await runCommand(undefined, "git", "rev-parse", "HEAD")).trim(); - - // Return to original directory - process.chdir(cwd); - - return { repo_name, commitHash }; -} - -async function uploadDirectoryToS3( - directoryPath: string, - s3BasePath: string, - workspace: string, -): Promise { - console.log(`Uploading ${directoryPath} -> ${s3BasePath}`); - - // Walk once into a flat task list so we can drive a bounded-concurrency pool. - const tasks: { localPath: string; s3Key: string }[] = []; - function walk(dir: string, s3Path: string) { - for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { - const fullPath = join(dir, entry.name); - const s3Key = s3Path ? `${s3Path}/${entry.name}` : entry.name; - if (entry.isDirectory()) { - walk(fullPath, s3Key); - } else if (entry.isFile()) { - tasks.push({ localPath: fullPath, s3Key }); - } - } - } - walk(directoryPath, s3BasePath); - console.log(`Discovered ${tasks.length} files to upload`); - - let nextIndex = 0; - let uploaded = 0; - let lastReport = 0; - async function worker() { - while (true) { - const idx = nextIndex++; - if (idx >= tasks.length) return; - const { localPath, s3Key } = tasks[idx]; - const fileContent = fs.readFileSync(localPath); - const blob = new Blob([fileContent], { type: 'application/octet-stream' }); - await wmillclient.HelpersService.gitRepoViewerFileUpload({ - workspace, - fileKey: s3Key, - requestBody: blob, - }); - uploaded++; - if (uploaded - lastReport >= 25 || uploaded === tasks.length) { - lastReport = uploaded; - console.log(`Uploaded ${uploaded} / ${tasks.length} files`); - } - } - } - await Promise.all( - Array.from({ length: Math.min(UPLOAD_CONCURRENCY, tasks.length) }, () => worker()) - ); - - // Marker is the LAST write — its presence is what the viewer checks for. - const markerKey = `${s3BasePath}/${CLONE_MARKER_FILE}`; - const markerBody = JSON.stringify({ - completed_at: new Date().toISOString(), - file_count: tasks.length, - }); - await wmillclient.HelpersService.gitRepoViewerFileUpload({ - workspace, - fileKey: markerKey, - requestBody: new Blob([markerBody], { type: 'application/json' }), - }); - console.log(`Wrote completion marker: ${markerKey}`); - - return tasks.length; -} - -function runCommand(secret_position: number | undefined, cmd: string, ...args: string[]): Promise { - const nargs = secret_position != undefined ? args.slice() : args; - if (secret_position && secret_position < 0) - secret_position = nargs.length - 1 + secret_position; - - let secret: string | undefined = undefined; - if (secret_position != undefined) { - nargs[secret_position] = "***"; - secret = args[secret_position]; - } - console.log(`Running shell command: '${cmd} ${nargs.join(" ")} ...'`); - - return new Promise((resolve, reject) => { - const process = spawn(cmd, args); - - let stdout = ''; - let stderr = ''; - - process.stdout.on('data', (data) => { - stdout += data.toString(); - }); - - process.stderr.on('data', (data) => { - stderr += data.toString(); - }); - - process.on('error', (error) => { - let errorString = error.toString(); - if (secret) errorString = errorString.replace(secret, "***"); - console.log(`Shell command FAILED: ${cmd}`, errorString); - const e = new Error( - `SH command '${cmd} ${nargs.join(" ")}' failed: ${errorString}` - ); - reject(e); - }); - - process.on('close', (code) => { - if (stdout.length > 0) { - console.log("Shell stdout:", stdout); - } - if (stderr.length > 0) { - console.log("Shell stderr:", stderr); - } - if (code === 0) { - console.log(`Shell command completed successfully: ${cmd}`); - resolve(stdout); - } else { - reject(new Error(`Command failed with code ${code}: ${stderr}`)); - } - }); - }); -} - -async function get_gh_app_token() { - const workspace = process.env["WM_WORKSPACE"]; - const jobToken = process.env["WM_TOKEN"]; - const baseUrl = - process.env["BASE_INTERNAL_URL"] ?? - process.env["BASE_URL"] ?? - "http://localhost:8000"; - const url = `${baseUrl}/api/w/${workspace}/github_app/token`; - - const response = await fetch(url, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: `Bearer ${jobToken}`, - }, - body: JSON.stringify({ job_token: jobToken }), - }); - - if (!response.ok) { - const errorBody = await response.text().catch(() => ""); - throw new Error(`GitHub App token error (${response.status}): ${errorBody || response.statusText}`); - } - const data = await response.json(); - return data.token; -} - -function prependTokenToGitHubUrl(gitHubUrl: string, installationToken: string) { - const url = new URL(gitHubUrl); - return `https://x-access-token:${installationToken}@${url.hostname}${url.pathname}`; -} diff --git a/docs/git-repo-viewer-hub-script.md b/docs/git-repo-viewer-hub-script.md index 7bfbe8160f..a1b5ea1482 100644 --- a/docs/git-repo-viewer-hub-script.md +++ b/docs/git-repo-viewer-hub-script.md @@ -1,7 +1,10 @@ # Git repo viewer — hub script -The hub script `clone_repo_and_upload_to_instance_storage` is published from -`windmill-integrations` and pinned in `frontend/src/lib/hubPaths.json` as +The hub script `clone_repo_and_upload_to_instance_storage` +([hub page](https://hub.windmill.dev/scripts/windmill/13968)) is published from +`windmill-integrations` +(`hub/windmill/scripts/action/13968_clone_repo_and_upload_to_instance_storage/script.ts`) +and pinned in `frontend/src/lib/hubPaths.json` as `cloneRepoToS3forGitRepoViewer`. Hub paths are exact version pins, so editing the script means publishing a new version and repointing that entry. @@ -23,6 +26,11 @@ The repo viewer in the Windmill app expects the hub script to: 3. **Write a completion marker** as the very last action of a successful run, so the API and frontend can distinguish a fully-populated S3 directory from a partial / interrupted upload. +4. **Follow symlinks that stay inside the checkout.** Both the git clone and + the archive extraction keep a repository's symlinks as links, and + `Dirent.isFile()` / `isDirectory()` are both false for a link, so a walk + that only checks those drops every linked file and directory from the + viewer. See [Symlinks](#symlinks). The marker file the frontend looks for is `.windmill_clone_complete` at the root of the per-commit directory: @@ -43,28 +51,74 @@ after the walk completes: ```ts const UPLOAD_CONCURRENCY = 16 const CLONE_MARKER_FILE = ".windmill_clone_complete" +const MAX_SYMLINKED_ENTRIES = 20_000 +const MAX_SYMLINKED_BYTES = 512 * 1024 * 1024 async function uploadDirectoryToS3( directoryPath: string, s3BasePath: string, workspace: string, -) { +): Promise { console.log(`Uploading ${directoryPath} -> ${s3BasePath}`) // Walk the directory once, producing a flat list of (localPath, s3Key) pairs. const tasks: { localPath: string; s3Key: string }[] = [] - function walk(dir: string, s3Path: string) { + const root = fs.realpathSync(directoryPath) + // Real paths of the directories being descended through. + const ancestors = new Set() + // What entries reached through a link have cost so far; see Symlinks below. + let symlinkedEntries = 0 + let symlinkedBytes = 0 + let symlinkBudgetSpent = false + function chargeSymlinkBudget(relPath: string, entries: number, bytes: number): boolean { + if (symlinkBudgetSpent) return false + symlinkedEntries += entries + symlinkedBytes += bytes + if (symlinkedEntries <= MAX_SYMLINKED_ENTRIES && symlinkedBytes <= MAX_SYMLINKED_BYTES) { + return true + } + symlinkBudgetSpent = true + console.log( + `Skipping ${relPath} and every symlinked entry after it: symlinks reach more than ` + + `${MAX_SYMLINKED_ENTRIES} entries or ${MAX_SYMLINKED_BYTES / 2 ** 20} MiB` + ) + return false + } + function walk(dir: string, relDir: string, viaLink: boolean) { + ancestors.add(dir) for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { - const fullPath = join(dir, entry.name) - const s3Key = s3Path ? `${s3Path}/${entry.name}` : entry.name - if (entry.isDirectory()) { - walk(fullPath, s3Key) - } else if (entry.isFile()) { - tasks.push({ localPath: fullPath, s3Key }) + const relPath = relDir ? `${relDir}/${entry.name}` : entry.name + const linked = viaLink || entry.isSymbolicLink() + if (linked && !chargeSymlinkBudget(relPath, 1, 0)) continue + let localPath = join(dir, entry.name) + if (entry.isSymbolicLink()) { + const link = fs.readlinkSync(localPath) + try { + localPath = fs.realpathSync(localPath) + } catch (e: any) { + console.log(`Skipping symlink ${relPath} -> ${link}: cannot resolve target (${e.code})`) + continue + } + if (localPath !== root && !localPath.startsWith(root + sep)) { + console.log(`Skipping symlink ${relPath} -> ${link}: target is outside the repository`) + continue + } + } + const stat = fs.statSync(localPath) + if (stat.isDirectory() && ancestors.has(localPath)) { + console.log(`Skipping ${relPath}: links back to a directory it is inside`) + continue + } + if (linked && stat.isFile() && !chargeSymlinkBudget(relPath, 0, stat.size)) continue + if (stat.isDirectory()) { + walk(localPath, relPath, linked) + } else if (stat.isFile()) { + tasks.push({ localPath, s3Key: `${s3BasePath}/${relPath}` }) } } + ancestors.delete(dir) } - walk(directoryPath, s3BasePath) + walk(root, "", false) console.log(`Discovered ${tasks.length} files to upload`) @@ -114,9 +168,40 @@ async function uploadDirectoryToS3( requestBody: new Blob([markerBody], { type: "application/json" }), }) console.log(`Wrote completion marker: ${markerKey}`) + + return tasks.length } ``` +## Symlinks + +A link is resolved with `realpathSync` and followed only when its target lies +inside the checkout's real path. A file target is uploaded under the link's own +path; a directory target is walked as if it sat there, so +`inventories/prod/group_vars -> ../../shared/group_vars` shows up in the viewer +with its files. Everything else is skipped and logged: + +- **A target outside the checkout.** The repository chooses the target, and the + checkout sits in the job's working directory next to the ssh key + `get_git_ssh_cmd` writes (`../ssh_id_priv_0`) and the job's `args.json`. A + link to one of those, or to `/proc/self/environ` with the caller's + `WM_TOKEN`, would put it in storage for every reader of the resource. This + is why the walk does not follow links the way `aws s3 sync` does. +- **A target that cannot be resolved**: a dangling link, or a link loop + (`ELOOP`). +- **A directory that is already being walked higher up** (`loop -> .`, + `up -> ..`). The guard holds the real paths of the current descent only, as + `find -L` does, not every directory seen so far: a directory reachable + through two links is uploaded under both paths, as the checkout presents it. +- **Anything reached through a link once the budget is spent.** Because a + directory can be reached along many paths, two links to the next directory + at each level double the tree, and a repository a few dozen links deep would + expand past what the job can hold in memory. Every entry reached through a + link counts against a budget of 20,000 entries and 512 MiB. It is charged + before the link is resolved, so links that end up skipped count too, and + neither their work nor their log lines can multiply. Past the budget, the rest + are skipped with one log line. The checkout's own files are always uploaded. + ## Notes for review - **Concurrency level**: 16 is a starting point; tune based on instance @@ -127,6 +212,12 @@ async function uploadDirectoryToS3( paths on retry, so a partial upload + retry naturally heals. Old commit directories from before this patch are unreachable through the UI but still consume storage; an instance admin can prune them manually if desired. +- **A new pin doesn't refresh commits already uploaded**: the viewer keys + storage on the commit hash (`gitrepos/{workspace}/{resource_path}/{commit_hash}/`) + and only checks that the marker exists. So a commit uploaded by an earlier + script version keeps that version's tree (hub/28905's had no symlinks) until + the repository's head moves to a new commit, or an admin deletes that + commit's directory. - **Error propagation**: keep the existing `try/catch` in `main` so an upload failure surfaces in the job result and is shown in the new viewer error banner. diff --git a/frontend/src/lib/hubPaths.json b/frontend/src/lib/hubPaths.json index 77c1f829ce..7439ad7ada 100644 --- a/frontend/src/lib/hubPaths.json +++ b/frontend/src/lib/hubPaths.json @@ -12,5 +12,5 @@ "discordReport": "hub/9085/discord", "smtpReport": "hub/28242/smtp", "appReport": "hub/28243/app-report", - "cloneRepoToS3forGitRepoViewer": "hub/28905/clone_repo_and_upload_to_instance_storage" + "cloneRepoToS3forGitRepoViewer": "hub/28968/clone_repo_and_upload_to_instance_storage" }