-
Notifications
You must be signed in to change notification settings - Fork 366
fix(setup-sourcebot): mount Vertex service-account credentials #1654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||||
|
|
||||||
| ## [Unreleased] | ||||||
|
|
||||||
| ### Fixed | ||||||
| - Fixed setup-sourcebot service-account credentials for Google Vertex providers by generating a read-only container mount and using its container path. | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: This new CHANGELOG entry omits the PR link that the documented convention requires. AGENTS.md/CLAUDE.md stipulate every entry under [Unreleased] end with a link Prompt for AI agents
Suggested change
|
||||||
|
|
||||||
| ## [5.1.13] - 2026-09-12 | ||||||
|
|
||||||
| ### Fixed | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,8 +22,10 @@ import { | |
| type EnvVars, | ||
| generateConnectionName, | ||
| generateSecret, | ||
| GOOGLE_APPLICATION_CREDENTIALS_CONTAINER_PATH, | ||
| INPUT_THEME, | ||
| note, | ||
| readOnlyBindMount, | ||
| } from './utils.js'; | ||
|
|
||
| const DOCKER_COMPOSE_BRANCH = 'main'; | ||
|
|
@@ -511,8 +513,13 @@ async function main() { | |
| } | ||
| } | ||
|
|
||
| const { models, env: modelEnv } = await collectModels(); | ||
| const { models, env: modelEnv, credentialHostPath } = await collectModels(); | ||
| Object.assign(allEnv, modelEnv); | ||
| if (credentialHostPath) { | ||
| // The host path must never be written to .env: Compose runs Sourcebot in | ||
| // a container, where only the stable container target is available. | ||
| allEnv.GOOGLE_APPLICATION_CREDENTIALS = GOOGLE_APPLICATION_CREDENTIALS_CONTAINER_PATH; | ||
| } | ||
|
|
||
| const authUrl = await input({ | ||
| message: 'What URL will Sourcebot be hosted at?', | ||
|
|
@@ -554,7 +561,7 @@ async function main() { | |
| } | ||
| } | ||
|
|
||
| if (localRepoIndex.size > 0 && existsSync('docker-compose.override.yml')) { | ||
| if ((localRepoIndex.size > 0 || credentialHostPath) && existsSync('docker-compose.override.yml')) { | ||
| const overwrite = await confirm({ | ||
| message: 'docker-compose.override.yml already exists. Overwrite?', | ||
| default: true, | ||
|
|
@@ -615,10 +622,16 @@ async function main() { | |
|
|
||
| const writtenFiles = ['config.json', '.env']; | ||
|
|
||
| if (localRepoIndex.size > 0) { | ||
| if (localRepoIndex.size > 0 || credentialHostPath) { | ||
| const mounts = [...localRepoIndex.entries()] | ||
| .sort((a, b) => a[1] - b[1]) | ||
| .map(([p, i]) => ` - ${p}:/repos/${i}:ro`); | ||
| .flatMap(([p, i]) => readOnlyBindMount(p, `/repos/${i}`)); | ||
| if (credentialHostPath) { | ||
| mounts.push(...readOnlyBindMount( | ||
| credentialHostPath, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: When the selected JSON is mode Prompt for AI agentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: When a credential filename contains Prompt for AI agents |
||
| GOOGLE_APPLICATION_CREDENTIALS_CONTAINER_PATH, | ||
| )); | ||
| } | ||
| const overrideYaml = [ | ||
| '# Generated by setup-sourcebot', | ||
| '# Merged with docker-compose.yml at `docker compose up` time.', | ||
|
|
@@ -644,7 +657,7 @@ async function main() { | |
| docsUrl: 'https://docs.sourcebot.dev/docs/configuration/environment-variables', | ||
| }, | ||
| 'docker-compose.override.yml': { | ||
| description: 'Mounts your local repositories into the Sourcebot container so they can be indexed. Merged with docker-compose.yml at `docker compose up` time.', | ||
| description: 'Mounts your local repositories and, when configured, Google Vertex credentials into the Sourcebot container. All generated bind mounts are read-only and merged with docker-compose.yml at `docker compose up` time.', | ||
| }, | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,8 @@ | ||
| import { confirm, input, password, select } from '@inquirer/prompts'; | ||
| import { select as searchSelect } from 'inquirer-select-pro'; | ||
| import { accessSync, constants, statSync } from 'node:fs'; | ||
| import { homedir } from 'node:os'; | ||
| import { join, resolve } from 'node:path'; | ||
| import type { | ||
| AmazonBedrockLanguageModel, | ||
| AzureLanguageModel, | ||
|
|
@@ -53,6 +56,32 @@ const PROVIDER_ID_OVERRIDES: Record<string, string> = { | |
| 'google-generative-ai': 'google', | ||
| }; | ||
|
|
||
| function expandHostPath(p: string): string { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: The new Prompt for AI agents |
||
| const trimmed = p.trim(); | ||
| if (trimmed.startsWith('~')) { | ||
| return resolve(join(homedir(), trimmed.slice(1))); | ||
| } | ||
| return resolve(trimmed); | ||
| } | ||
|
|
||
| function isReadableFile(path: string): boolean { | ||
| try { | ||
| return statSync(path).isFile() && accessSync(path, constants.R_OK) === undefined; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| export function validateCredentialPath(value: string): true | string { | ||
| if (!value?.trim()) { | ||
| return 'Credentials path is required'; | ||
| } | ||
| const path = expandHostPath(value); | ||
| return isReadableFile(path) | ||
| ? true | ||
| : 'Credentials file must exist and be readable'; | ||
| } | ||
|
|
||
| let catalogPromise: Promise<ModelsDevCatalog | null> | null = null; | ||
|
|
||
| async function loadCatalog(): Promise<ModelsDevCatalog | null> { | ||
|
|
@@ -164,6 +193,7 @@ async function collectModelConfig( | |
| provider: Provider, | ||
| model: string, | ||
| env: EnvVars, | ||
| credentialPath: { value?: string }, | ||
| ): Promise<LanguageModel> { | ||
| switch (provider) { | ||
| case 'anthropic': | ||
|
|
@@ -284,9 +314,11 @@ async function collectModelConfig( | |
| if (!useAppDefault) { | ||
| if (!env['GOOGLE_APPLICATION_CREDENTIALS']) { | ||
| env['GOOGLE_APPLICATION_CREDENTIALS'] = await input({ | ||
| message: 'Path to service account credentials JSON (stored locally in .env as GOOGLE_APPLICATION_CREDENTIALS)', | ||
| validate: (v) => !v?.trim() ? 'Credentials path is required' : true, | ||
| message: 'Path to service account credentials JSON (mounted read-only into the Sourcebot container)', | ||
| validate: validateCredentialPath, | ||
| }); | ||
| env['GOOGLE_APPLICATION_CREDENTIALS'] = expandHostPath(env['GOOGLE_APPLICATION_CREDENTIALS']); | ||
| credentialPath.value = env['GOOGLE_APPLICATION_CREDENTIALS']; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: When the two Vertex providers require different service accounts, Prompt for AI agents |
||
| } | ||
| config.credentials = { env: 'GOOGLE_APPLICATION_CREDENTIALS' }; | ||
| } | ||
|
Comment on lines
314
to
324
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift Support distinct credentials for both Vertex providers
🤖 Prompt for AI Agents |
||
|
|
@@ -295,9 +327,10 @@ async function collectModelConfig( | |
| } | ||
| } | ||
|
|
||
| export async function collectModels(): Promise<{ models: LanguageModel[]; env: EnvVars }> { | ||
| export async function collectModels(): Promise<{ models: LanguageModel[]; env: EnvVars; credentialHostPath?: string }> { | ||
| const models: LanguageModel[] = []; | ||
| const env: EnvVars = {}; | ||
| const credentialPath: { value?: string } = {}; | ||
|
|
||
| note( | ||
| [ | ||
|
|
@@ -317,7 +350,7 @@ export async function collectModels(): Promise<{ models: LanguageModel[]; env: E | |
| }); | ||
|
|
||
| if (!wantsAI) { | ||
| return { models, env }; | ||
| return { models, env, credentialHostPath: credentialPath.value }; | ||
| } | ||
|
|
||
| // eslint-disable-next-line no-constant-condition | ||
|
|
@@ -354,7 +387,7 @@ export async function collectModels(): Promise<{ models: LanguageModel[]; env: E | |
| validate: (v) => !v?.trim() ? 'Model name is required' : true, | ||
| }); | ||
|
|
||
| const config = await collectModelConfig(provider, model, env); | ||
| const config = await collectModelConfig(provider, model, env, credentialPath); | ||
|
|
||
| const displayName = (await input({ | ||
| message: 'Display name (optional, press enter to skip)', | ||
|
|
@@ -374,5 +407,5 @@ export async function collectModels(): Promise<{ models: LanguageModel[]; env: E | |
| } | ||
| } | ||
|
|
||
| return { models, env }; | ||
| return { models, env, credentialHostPath: credentialPath.value }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import assert from 'node:assert/strict'; | ||
| import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; | ||
| import { tmpdir } from 'node:os'; | ||
| import { join } from 'node:path'; | ||
| import test from 'node:test'; | ||
| import { validateCredentialPath } from '../src/models.js'; | ||
| import { | ||
| GOOGLE_APPLICATION_CREDENTIALS_CONTAINER_PATH, | ||
| readOnlyBindMount, | ||
| } from '../src/utils.js'; | ||
|
|
||
| test('credential path validation requires a readable file', () => { | ||
| const root = mkdtempSync(join(tmpdir(), 'setup-sourcebot-credentials-')); | ||
| const credentialFile = join(root, 'service account.json'); | ||
| writeFileSync(credentialFile, '{}'); | ||
|
|
||
| try { | ||
| assert.equal(validateCredentialPath(''), 'Credentials path is required'); | ||
| assert.equal(validateCredentialPath(join(root, 'missing.json')), 'Credentials file must exist and be readable'); | ||
| assert.equal(validateCredentialPath(root), 'Credentials file must exist and be readable'); | ||
| assert.equal(validateCredentialPath(credentialFile), true); | ||
|
|
||
| chmodSync(credentialFile, 0o000); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: The unreadable-file assertion only holds for non-root users. Prompt for AI agents |
||
| assert.equal(validateCredentialPath(credentialFile), 'Credentials file must exist and be readable'); | ||
|
Comment on lines
+23
to
+24
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- test ---'
cat -n packages/setupWizard/test/credentials.test.ts
printf '%s\n' '--- implementation references ---'
rg -n -C 8 'validateCredentialPath|accessSync|chmodSync' packages/setupWizard
printf '%s\n' '--- package metadata ---'
fd -i 'package.json|vitest.config.*|jest.config.*' packages/setupWizard
for f in $(fd -i 'package.json|vitest.config.*|jest.config.*' packages/setupWizard); do
printf '\n--- %s ---\n' "$f"
cat -n "$f"
doneRepository: sourcebot-dev/sourcebot Length of output: 10695 🤖 get_repo_knowledge executed:
Length of output: 7568 🏁 Script executed: #!/bin/bash
set -e
node - <<'JS'
'use strict';
const { accessSync, chmodSync, constants, mkdtempSync, rmSync, writeFileSync } = require('node:fs');
const { tmpdir } = require('node:os');
const { join } = require('node:path');
const root = mkdtempSync(join(tmpdir(), 'permission-probe-'));
const file = join(root, 'file');
writeFileSync(file, '{}');
try {
chmodSync(file, 0o000);
let accessResult = 'throws';
try {
accessSync(file, constants.R_OK);
accessResult = 'succeeds';
} catch {}
console.log(JSON.stringify({
platform: process.platform,
uid: typeof process.getuid === 'function' ? process.getuid() : null,
accessAfterMode000: accessResult,
}));
} finally {
chmodSync(file, 0o644);
rmSync(root, { recursive: true, force: true });
}
JSRepository: sourcebot-dev/sourcebot Length of output: 223 🌐 Web query:
💡 Result: In Node.js, the behavior of fs.accessSync and fs.chmodSync regarding mode bits on Windows is significantly restricted compared to POSIX systems [1]. fs.accessSync The fs.accessSync(path, mode) method tests a user's permissions [2]. On Windows, access control policies (ACLs) can limit access to a file or directory; however, fs.accessSync does not check these ACLs [1]. Consequently, it may report that a path is accessible even if the user is restricted by an ACL [1]. Regarding the mode constants: - F_OK, R_OK, W_OK, and X_OK are supported [3]. - X_OK (execute permission) has no effect on Windows and behaves identically to F_OK [3]. fs.chmodSync The fs.chmodSync(path, mode) method is used to change the permissions of a file [4][5]. On Windows, its functionality is limited by the operating system's underlying file system capabilities [1]: - Only the write permission can be changed [1]. - The distinction between owner, group, and others is not implemented [1]. - Attempting to use mode bits to modify read or execute permissions will not have the intended effect, as these are not supported on Windows [1]. Because of these limitations, using fs.chmodSync on Windows to control granular access permissions is ineffective [1]. Developers are generally advised to rely on standard Windows file system permissions managed through the operating system rather than Node.js mode bits when working in a Windows environment [1]. Citations:
Make the unreadable-file assertion portable.
🤖 Prompt for AI Agents |
||
| } finally { | ||
| chmodSync(credentialFile, 0o644); | ||
| rmSync(root, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
|
|
||
| test('read-only bind mounts preserve special host paths and the container target', () => { | ||
| const source = '/tmp/repos with spaces/#root/service account "vertex".json'; | ||
| assert.deepEqual(readOnlyBindMount(source, GOOGLE_APPLICATION_CREDENTIALS_CONTAINER_PATH), [ | ||
| ' - type: bind', | ||
| ` source: ${JSON.stringify(source)}`, | ||
| ` target: ${JSON.stringify(GOOGLE_APPLICATION_CREDENTIALS_CONTAINER_PATH)}`, | ||
| ' read_only: true', | ||
| ]); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the pull request reference.
Append the PR link in the required
[#<id>](https://github.com/sourcebot-dev/sourcebot/pull/<id>)format once the PR ID is available.As per coding guidelines, “entries must include the GitHub pull request id at the end of the line.”
🤖 Prompt for AI Agents
Source: Coding guidelines