From 66222019d4372a04a892887aff46ed97aa668fd3 Mon Sep 17 00:00:00 2001 From: "A.R." Date: Fri, 12 Jun 2026 10:03:23 +0300 Subject: [PATCH 01/19] fix(daemon): create daemon.lock with 0600 perms + Windows ACL, sanitize icacls principal The lockfile carries the plaintext bearer token but was created with default permissions (world-readable on Unix). acquire() now opens with mode 0o600, replace() stages via safeAtomicWriteFileSync at 0o600, and both apply the same Windows ACL restriction daemon.token already used. The LSP port_lsp writer stages its temp file at 0o600 so its atomic rename does not undo the hardening. applyPrivatePermissions is exported from token.js and now sanitizes USERNAME/USERDOMAIN before building the icacls grant principal. Co-Authored-By: Claude Fable 5 --- packages/lsp-server/src/lockfile-writer.ts | 5 ++++- packages/mcp-server/src/daemon/lockfile.js | 14 +++++++++----- packages/mcp-server/src/daemon/token.js | 18 +++++++++++++++--- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/packages/lsp-server/src/lockfile-writer.ts b/packages/lsp-server/src/lockfile-writer.ts index db5d74c..b9ad023 100644 --- a/packages/lsp-server/src/lockfile-writer.ts +++ b/packages/lsp-server/src/lockfile-writer.ts @@ -21,7 +21,10 @@ export function writeLspPort(lockPath: string, port: number): boolean { const updated = { ...existing, port_lsp: port }; const tempPath = `${lockPath}.lsp.tmp`; // Must be same directory as lockPath for atomic rename mkdirSync(dirname(lockPath), { recursive: true }); - writeFileSync(tempPath, JSON.stringify(updated, null, 2) + '\n', 'utf8'); + // mode 0o600 — daemon.lock carries the bearer token; rename preserves the + // temp file's permissions, so an unrestricted temp would undo the daemon's + // own permission hardening (lockfile.js writes it 0o600). + writeFileSync(tempPath, JSON.stringify(updated, null, 2) + '\n', { encoding: 'utf8', mode: 0o600 }); renameSync(tempPath, lockPath); // atomic replace (lockfile.js lines 121-123 pattern) return true; } diff --git a/packages/mcp-server/src/daemon/lockfile.js b/packages/mcp-server/src/daemon/lockfile.js index 50a68f0..3fce04f 100644 --- a/packages/mcp-server/src/daemon/lockfile.js +++ b/packages/mcp-server/src/daemon/lockfile.js @@ -1,6 +1,8 @@ -import { closeSync, existsSync, mkdirSync, openSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs'; +import { closeSync, existsSync, mkdirSync, openSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { getHomeDir } from '../paths.js'; +import { safeAtomicWriteFileSync } from '../safe-write.js'; +import { applyPrivatePermissions } from './token.js'; /** * @typedef {Object} DaemonLockRecord @@ -27,7 +29,9 @@ export function acquire(record, options = {}) { for (let attempt = 0; attempt < 2; attempt++) { let fd; try { - fd = openSync(lockPath, 'wx'); + // 0o600 — the lockfile carries the plaintext bearerToken; without an + // explicit mode it would be created world-readable (default 0o644). + fd = openSync(lockPath, 'wx', 0o600); } catch (error) { if (isExistsError(error)) { if (attempt === 0 && tryReclaimStale(lockPath)) { @@ -49,6 +53,7 @@ export function acquire(record, options = {}) { } } + applyPrivatePermissions(lockPath); return true; } @@ -117,9 +122,8 @@ export function replace(record, options = {}) { } mkdirSync(dirname(lockPath), { recursive: true }); - const tempPath = `${lockPath}.tmp`; - writeFileSync(tempPath, serialize(normalized), 'utf8'); - renameSync(tempPath, lockPath); + safeAtomicWriteFileSync(lockPath, serialize(normalized), { encoding: 'utf8', mode: 0o600 }); + applyPrivatePermissions(lockPath); return true; } diff --git a/packages/mcp-server/src/daemon/token.js b/packages/mcp-server/src/daemon/token.js index 3388525..e17b1cb 100644 --- a/packages/mcp-server/src/daemon/token.js +++ b/packages/mcp-server/src/daemon/token.js @@ -101,7 +101,12 @@ function normalizeRecord(value) { }; } -function applyPrivatePermissions(tokenPath) { +/** + * Restrict a secret-bearing file to the current user. Best-effort and + * non-fatal on every platform — shared by daemon.token and daemon.lock, + * both of which hold the plaintext bearer token. + */ +export function applyPrivatePermissions(tokenPath) { if (process.platform === 'win32') { restrictWindowsAcl(tokenPath); } else { @@ -109,10 +114,17 @@ function applyPrivatePermissions(tokenPath) { } } +// icacls parses its `principal:permissions` argument itself; characters like +// `(`, `)`, `,`, `:` in an unsanitized USERNAME/USERDOMAIN could corrupt the +// grant (see extension/src/mcp/secure-permissions.ts for the same defense). +function sanitizeAclPart(value) { + return (value ?? '').replace(/[^A-Za-z0-9 ._-]/g, ''); +} + function restrictWindowsAcl(tokenPath) { try { - const username = process.env.USERNAME; - const domain = process.env.USERDOMAIN; + const username = sanitizeAclPart(process.env.USERNAME); + const domain = sanitizeAclPart(process.env.USERDOMAIN); const target = domain && username ? `${domain}\\${username}` : username; if (!target) throw new Error('Cannot resolve Windows username'); const result = spawnSync('icacls', [tokenPath, '/inheritance:r', '/grant:r', `${target}:(R,W)`], { From 17249757ad8827ef807127c3b2898b451aa843af Mon Sep 17 00:00:00 2001 From: "A.R." Date: Fri, 12 Jun 2026 10:03:36 +0300 Subject: [PATCH 02/19] fix(auth): deliver login credentials over fork IPC instead of child env AIRTABLE_EMAIL/PASSWORD/OTP_SECRET were placed in the login-runner child environment, visible via /proc//environ on Linux and captured in core dumps. The runner now requests credentials over the fork() IPC channel (request-credentials -> credentials handshake); env remains a documented fallback for standalone use. The VS Code MCP stdio definition (registration.ts) still uses env because VS Code owns that spawn and env is the only available channel there. Co-Authored-By: Claude Fable 5 --- packages/extension/src/mcp/auth-manager.ts | 61 +++++++++++++++---- packages/mcp-server/src/login-runner.js | 70 +++++++++++++++++++--- 2 files changed, 113 insertions(+), 18 deletions(-) diff --git a/packages/extension/src/mcp/auth-manager.ts b/packages/extension/src/mcp/auth-manager.ts index 101e093..0ef6482 100644 --- a/packages/extension/src/mcp/auth-manager.ts +++ b/packages/extension/src/mcp/auth-manager.ts @@ -238,10 +238,10 @@ export class AuthManager implements vscode.Disposable { } /** - * Get credentials as env vars for passing to child processes. + * Read stored credentials from SecretStorage. * Returns undefined if no credentials are stored. */ - async getCredentialsEnv(): Promise | undefined> { + async getCredentials(): Promise<{ email: string; password: string; otpSecret?: string } | undefined> { // D-02: ensure daemon is running before handing off credentials if (getSettings().mcp.useDaemon && this._daemonManager) { await this._daemonManager.ensureDaemon(); @@ -250,12 +250,26 @@ export class AuthManager implements vscode.Disposable { const password = await this.getPassword(); if (!email || !password) return undefined; + const otp = await this.getOtpSecret(); + return { email, password, ...(otp ? { otpSecret: otp } : {}) }; + } + + /** + * Get credentials as env vars. ONLY for the VS Code MCP stdio definition + * (registration.ts), where VS Code owns the spawn and env is the only + * channel. Helper scripts we fork ourselves receive credentials over the + * IPC channel instead (_spawnScript) so they never appear in the child's + * environment (/proc//environ, process listings, core dumps). + */ + async getCredentialsEnv(): Promise | undefined> { + const creds = await this.getCredentials(); + if (!creds) return undefined; + const env: Record = { - AIRTABLE_EMAIL: email, - AIRTABLE_PASSWORD: password, + AIRTABLE_EMAIL: creds.email, + AIRTABLE_PASSWORD: creds.password, }; - const otp = await this.getOtpSecret(); - if (otp) env.AIRTABLE_OTP_SECRET = otp; + if (creds.otpSecret) env.AIRTABLE_OTP_SECRET = creds.otpSecret; return env; } @@ -328,16 +342,17 @@ export class AuthManager implements vscode.Disposable { const loginMode = this._getLoginMode(); if (loginMode === 'auto') { - const creds = await this.getCredentialsEnv(); + const creds = await this.getCredentials(); if (!creds) { this._updateState({ status: 'error', error: 'No credentials stored. Save credentials first.' }); return this._state; } this._updateState({ status: 'logging-in' }); try { + // Credentials go over the IPC channel, never the child environment. const result = await this._spawnScript('login-runner.mjs', { - ...creds, ...this._browserEnv(), ...this._profileEnv(), - }); + ...this._browserEnv(), ...this._profileEnv(), + }, undefined, creds); const now = new Date().toISOString(); if (result.ok) { this._updateState({ status: 'valid', userId: result.userId || undefined, lastLogin: now, lastChecked: now, error: undefined }); @@ -513,8 +528,17 @@ export class AuthManager implements vscode.Disposable { /** * Spawn a bundled MCP helper script as a child process. * Returns parsed JSON from stdout. + * + * `credentials` are delivered over the fork() IPC channel on the child's + * request — never via env, which is world-visible on Linux through + * /proc//environ and survives in core dumps. */ - private _spawnScript(scriptName: string, extraEnv?: Record, timeoutMs = 120_000): Promise { + private _spawnScript( + scriptName: string, + extraEnv?: Record, + timeoutMs = 120_000, + credentials?: { email: string; password: string; otpSecret?: string }, + ): Promise { return new Promise((resolve, reject) => { const scriptPath = path.join(this.extensionPath, 'dist', 'mcp', scriptName); const nodeModulesPath = path.join(this.extensionPath, 'dist', 'node_modules'); @@ -530,6 +554,23 @@ export class AuthManager implements vscode.Disposable { execArgv: ['--experimental-vm-modules'], }); + if (credentials) { + child.on('message', (msg: unknown) => { + if ((msg as { type?: string } | null)?.type === 'request-credentials') { + try { + child.send({ + type: 'credentials', + email: credentials.email, + password: credentials.password, + otpSecret: credentials.otpSecret ?? null, + }); + } catch { + // Child exited between request and reply — close handler reports it + } + } + }); + } + let stdout = ''; let stderr = ''; diff --git a/packages/mcp-server/src/login-runner.js b/packages/mcp-server/src/login-runner.js index 21c582b..7b6f7dd 100644 --- a/packages/mcp-server/src/login-runner.js +++ b/packages/mcp-server/src/login-runner.js @@ -3,14 +3,20 @@ * Programmatic login runner for the VS Code extension. * * Same flow as login.js but designed for non-interactive use: - * - Reads credentials from environment variables only (no CLI args for security) + * - Receives credentials over the fork() IPC channel when spawned by the + * extension (preferred — keeps secrets out of the child environment), + * falling back to environment variables for standalone use * - Outputs structured JSON to stdout * - Uses exit codes for success/failure * + * IPC protocol (when process.send is available): + * child → parent: { type: 'request-credentials' } + * parent → child: { type: 'credentials', email, password, otpSecret } + * * Environment variables: - * AIRTABLE_EMAIL — (required) Airtable account email - * AIRTABLE_PASSWORD — (required) Airtable account password - * AIRTABLE_OTP_SECRET — (optional) TOTP 2FA base32 secret + * AIRTABLE_EMAIL — (fallback) Airtable account email + * AIRTABLE_PASSWORD — (fallback) Airtable account password + * AIRTABLE_OTP_SECRET — (optional fallback) TOTP 2FA base32 secret * AIRTABLE_PROFILE — (optional) profile dir name (default: .chrome-profile) * AIRTABLE_BROWSER_CHANNEL — (optional) patchright channel (chrome|msedge|chromium) * AIRTABLE_BROWSER_PATH — (optional) absolute path to browser executable @@ -68,17 +74,65 @@ function generateTOTP(secretBase32) { return totp.generate(); } +/** + * Ask the parent process for credentials over the IPC channel. + * Resolves null when no parent answers within the timeout (standalone run, + * or a parent that doesn't speak the protocol) — caller falls back to env. + */ +function requestCredentialsOverIpc(timeoutMs = 10_000) { + return new Promise((resolve) => { + const onMessage = (msg) => { + if (msg && msg.type === 'credentials') { + cleanup(); + resolve({ + email: msg.email || null, + password: msg.password || null, + otpSecret: msg.otpSecret || null, + }); + } + }; + const timer = setTimeout(() => { + cleanup(); + resolve(null); + }, timeoutMs); + const cleanup = () => { + clearTimeout(timer); + process.removeListener('message', onMessage); + }; + process.on('message', onMessage); + try { + process.send({ type: 'request-credentials' }); + } catch { + cleanup(); + resolve(null); + } + }); +} + async function main() { - const email = process.env.AIRTABLE_EMAIL; - const password = process.env.AIRTABLE_PASSWORD; - const otpSecret = process.env.AIRTABLE_OTP_SECRET || null; + let email = process.env.AIRTABLE_EMAIL; + let password = process.env.AIRTABLE_PASSWORD; + let otpSecret = process.env.AIRTABLE_OTP_SECRET || null; + + // IPC-first: when spawned via fork() the parent holds the credentials and + // sends them on request, so they never enter this process's environment. + if ((!email || !password) && typeof process.send === 'function') { + console.error('[login-runner] Requesting credentials over IPC...'); + const ipcCreds = await requestCredentialsOverIpc(); + if (ipcCreds) { + email = ipcCreds.email || email; + password = ipcCreds.password || password; + otpSecret = ipcCreds.otpSecret || otpSecret; + } + } + const { getProfileDir } = await import('./paths.js'); const profileDir = getProfileDir(); const browserChannel = process.env.AIRTABLE_BROWSER_CHANNEL || 'chrome'; const browserPath = process.env.AIRTABLE_BROWSER_PATH || undefined; if (!email || !password) { - output({ ok: false, error: 'AIRTABLE_EMAIL and AIRTABLE_PASSWORD environment variables are required' }); + output({ ok: false, error: 'Credentials required: send them over IPC (fork) or set AIRTABLE_EMAIL and AIRTABLE_PASSWORD' }); process.exit(1); } From d22fff0016d55c77e649d9474f0457d760a6511c Mon Sep 17 00:00:00 2001 From: "A.R." Date: Fri, 12 Jun 2026 10:03:36 +0300 Subject: [PATCH 03/19] fix(ci): pin vsce/ovsx exactly and validate registry version replies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit vsce and ovsx run with VSCE_PAT/OVSX_PAT/NODE_AUTH_TOKEN in scope, so the release workflow must not auto-pull whatever latest is on npm: pin @vscode/vsce@3.9.2 + ovsx@1.0.1 and invoke via npx --no-install. Marketplace/npm version queries are external input interpolated into node -e version-bump scripts — accept strict semver only, fall back to 0.0.0 with a workflow warning otherwise. Quote OVSX_PAT. Co-Authored-By: Claude Fable 5 --- .github/workflows/release.yml | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index feb975a..eac1ce9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -54,7 +54,9 @@ jobs: registry-url: 'https://registry.npmjs.org' - run: pnpm install --frozen-lockfile - - run: pnpm install -wD @vscode/vsce ovsx + # Exact pins — these tools run with VSCE_PAT/OVSX_PAT/NODE_AUTH_TOKEN in + # scope, so a compromised latest release must not be auto-pulled here. + - run: pnpm install -wD @vscode/vsce@3.9.2 ovsx@1.0.1 # ── Compute versions ─────────────────────────────────────── # For each target, we check both package.json AND the registry. @@ -69,12 +71,19 @@ jobs: LOCAL=$(node -p "require('./$PKG').version") # Query VS Code Marketplace for the latest published version - PUBLISHED=$(npx vsce show Nskha.airtable-formula --json 2>/dev/null \ + PUBLISHED=$(npx --no-install vsce show Nskha.airtable-formula --json 2>/dev/null \ | node -e "let d='';process.stdin.on('data',c=>d+=c);process.stdin.on('end',()=>{ try{console.log(JSON.parse(d).versions[0].version)} catch{console.log('0.0.0')} })" || echo "0.0.0") + # The Marketplace reply is external input that gets interpolated into + # node -e below — accept strict semver only. + if ! [[ "$PUBLISHED" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "::warning::Marketplace returned non-semver version '${PUBLISHED}' — ignoring" + PUBLISHED="0.0.0" + fi + echo "Local: ${LOCAL}, Marketplace: ${PUBLISHED}" # Pick the higher version as the base for bumping @@ -118,6 +127,12 @@ jobs: # Query npm for the latest published version PUBLISHED=$(npm view airtable-user-mcp version 2>/dev/null || echo "0.0.0") + # External input interpolated into node -e below — strict semver only + if ! [[ "$PUBLISHED" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "::warning::npm returned non-semver version '${PUBLISHED}' — ignoring" + PUBLISHED="0.0.0" + fi + echo "Local: ${LOCAL}, npm: ${PUBLISHED}" # Pick the higher version as the base for bumping @@ -159,6 +174,12 @@ jobs: # Query npm for the latest published version PUBLISHED=$(npm view airtable-user-lsp version 2>/dev/null || echo "0.0.0") + # External input interpolated into node -e below — strict semver only + if ! [[ "$PUBLISHED" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "::warning::npm returned non-semver version '${PUBLISHED}' — ignoring" + PUBLISHED="0.0.0" + fi + echo "Local: ${LOCAL}, npm: ${PUBLISHED}" # Pick the higher version as the base for bumping @@ -226,7 +247,7 @@ jobs: # Package cd packages/extension - npx vsce package --no-dependencies + npx --no-install vsce package --no-dependencies VSIX=$(ls *.vsix | head -1) echo "file=packages/extension/${VSIX}" >> $GITHUB_OUTPUT echo "Packaged: ${VSIX}" @@ -236,7 +257,7 @@ jobs: if: | !inputs.dry_run && (inputs.target == 'extension' || inputs.target == 'both') - run: npx vsce publish --packagePath "${{ steps.vsix.outputs.file }}" + run: npx --no-install vsce publish --packagePath "${{ steps.vsix.outputs.file }}" env: VSCE_PAT: ${{ secrets.VSCE_PAT }} @@ -245,7 +266,7 @@ jobs: !inputs.dry_run && (inputs.target == 'extension' || inputs.target == 'both') run: | - npx ovsx publish "${{ steps.vsix.outputs.file }}" --pat $OVSX_PAT + npx --no-install ovsx publish "${{ steps.vsix.outputs.file }}" --pat "$OVSX_PAT" # Verify the version actually landed (Open VSX indexes asynchronously) VERSION="${{ steps.ext_version.outputs.next }}" From e631de85a671037dd5932bd089781a80b321b07b Mon Sep 17 00:00:00 2001 From: "A.R." Date: Fri, 12 Jun 2026 10:03:54 +0300 Subject: [PATCH 04/19] fix(build): refuse symlinks escaping workspace node_modules in VSIX dep copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cpSync with dereference:true follows every symlink in the copied packages and writes the target into the VSIX — a trojanized dependency shipping a symlink to ~/.ssh or CI credentials would have the target silently included in the published artifact. Walk each package tree before copying (cycle-safe, descending through directory-symlink targets) and fail the build if any symlink resolves outside the workspace node_modules tree, where all legitimate pnpm links live. Co-Authored-By: Claude Fable 5 --- scripts/prepare-package-deps.mjs | 68 +++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/scripts/prepare-package-deps.mjs b/scripts/prepare-package-deps.mjs index f938126..0244795 100644 --- a/scripts/prepare-package-deps.mjs +++ b/scripts/prepare-package-deps.mjs @@ -12,8 +12,8 @@ * using the mcp-server root as a paths hint, which transparently walks the * symlinked store and returns the real on-disk path. */ -import { cpSync, existsSync, mkdirSync, readFileSync, rmSync } from 'node:fs'; -import { dirname, join } from 'node:path'; +import { cpSync, existsSync, lstatSync, mkdirSync, readFileSync, readdirSync, realpathSync, rmSync } from 'node:fs'; +import { dirname, join, sep } from 'node:path'; import { fileURLToPath } from 'node:url'; import { createRequire } from 'node:module'; @@ -68,6 +68,68 @@ function resolvePackageRoot(packageName) { return undefined; } +// ── Symlink-escape guard ───────────────────────────────────────────────── +// `cpSync(..., { dereference: true })` follows EVERY symlink in the tree and +// copies its target into the VSIX. A trojanized npm package could ship a +// symlink pointing at ~/.ssh, CI credentials, etc., and the target file would +// silently end up inside the published artifact. Before copying, walk the +// tree (following directory symlinks, cycle-safe) and require every symlink +// to resolve inside the workspace node_modules tree — pnpm's legitimate +// nested-dependency links all resolve into /node_modules/.pnpm. + +const workspaceRoot = realpathSync(join(__dirname, '..')); + +// Windows paths are case-insensitive; normalize before prefix comparison. +const normalizeForCompare = (p) => (process.platform === 'win32' ? p.toLowerCase() : p); +const isWithin = (child, parent) => { + const c = normalizeForCompare(child); + const p = normalizeForCompare(parent); + return c === p || c.startsWith(p + sep); +}; + +function assertSafeSymlinks(rootDir) { + const allowedRoots = [realpathSync(rootDir), join(workspaceRoot, 'node_modules')]; + const visited = new Set(); + const stack = [realpathSync(rootDir)]; + + while (stack.length > 0) { + const dir = stack.pop(); + const dirKey = normalizeForCompare(dir); + if (visited.has(dirKey)) continue; + visited.add(dirKey); + + let entries; + try { entries = readdirSync(dir, { withFileTypes: true }); } catch { continue; } + + for (const entry of entries) { + const entryPath = join(dir, entry.name); + let stat; + try { stat = lstatSync(entryPath); } catch { continue; } + + if (stat.isSymbolicLink()) { + let resolved; + try { + resolved = realpathSync(entryPath); + } catch { + throw new Error(`Refusing to package: broken symlink ${entryPath}`); + } + if (!allowedRoots.some((root) => isWithin(resolved, root))) { + throw new Error( + `Refusing to package: symlink escapes workspace node_modules:\n ${entryPath}\n → ${resolved}` + ); + } + // dereference:true copies the target's contents too — keep walking + // through it so a second-level symlink can't smuggle files out. + try { + if (lstatSync(resolved).isDirectory()) stack.push(resolved); + } catch { /* target vanished — cpSync will surface it */ } + } else if (stat.isDirectory()) { + stack.push(entryPath); + } + } + } +} + for (const packageName of packagesToCopy) { const realSource = resolvePackageRoot(packageName); if (!realSource) { @@ -79,6 +141,8 @@ for (const packageName of packagesToCopy) { continue; } + assertSafeSymlinks(realSource); + const target = join(extensionNodeModules, packageName); // dereference: true — follow symlinks and copy real files, required for // pnpm's symlinked store to produce a self-contained VSIX. From 0325a80b2b9efc9837836126a1bb147572417421 Mon Sep 17 00:00:00 2001 From: "A.R." Date: Fri, 12 Jun 2026 10:03:55 +0300 Subject: [PATCH 05/19] fix(mcp): fail closed to read-only on unknown tool profile An unrecognized activeProfile in tools-config.json (hand-edited or corrupted) silently fell back to the full profile, enabling all 66 tools including destructive ones. It now falls back to the read-only set and logs a warning to stderr. tools-config.json is also written with mode 0o600. Adds a regression test. Co-Authored-By: Claude Fable 5 --- packages/mcp-server/src/tool-config.js | 17 ++++++++++++++--- .../mcp-server/test/test-tool-config.test.js | 13 +++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/packages/mcp-server/src/tool-config.js b/packages/mcp-server/src/tool-config.js index 54e5b05..4be83c4 100644 --- a/packages/mcp-server/src/tool-config.js +++ b/packages/mcp-server/src/tool-config.js @@ -209,7 +209,9 @@ export class ToolConfigManager { const tmp = `${configFile}.${randomBytes(6).toString('hex')}.tmp`; try { try { - await writeFile(tmp, JSON.stringify(this._config, null, 2), 'utf8'); + // 0o600 — this file gates which (potentially destructive) tools are + // exposed; don't leave it writable through lax default permissions. + await writeFile(tmp, JSON.stringify(this._config, null, 2), { encoding: 'utf8', mode: 0o600 }); await rename(tmp, configFile); } catch (err) { // Best-effort cleanup — rename may have happened before a later failure @@ -268,8 +270,17 @@ export class ToolConfigManager { const def = BUILTIN_PROFILES[profile]; if (!def) { - // Unknown profile → fall back to full - return new Set(Object.keys(TOOL_CATEGORIES)); + // Unknown profile → fail CLOSED to read-only. switchProfile() validates + // names, so this only happens when tools-config.json was hand-edited or + // tampered with — silently exposing destructive tools in that state + // would defeat the gating the user configured. + console.error(`[tool-config] Unknown activeProfile "${profile}" — failing closed to read-only`); + const readOnly = new Set(BUILTIN_PROFILES['read-only'].categories); + const enabled = new Set(); + for (const [tool, category] of Object.entries(TOOL_CATEGORIES)) { + if (readOnly.has(category)) enabled.add(tool); + } + return enabled; } const cats = new Set(def.categories); diff --git a/packages/mcp-server/test/test-tool-config.test.js b/packages/mcp-server/test/test-tool-config.test.js index eed41d2..1b034fe 100644 --- a/packages/mcp-server/test/test-tool-config.test.js +++ b/packages/mcp-server/test/test-tool-config.test.js @@ -126,6 +126,19 @@ describe('ToolConfigManager', () => { const enabled = mgr.enabledToolNames(); assert.equal(enabled.size, 66); }); + + it('unknown profile fails closed to read-only', async () => { + // switchProfile() rejects unknown names, so simulate a hand-edited / + // tampered tools-config.json by mutating loaded state directly. + mgr._config.activeProfile = 'totally-bogus'; + const enabled = mgr.enabledToolNames(); + assert.equal(enabled.size, 12, 'must match the read-only tool count'); + assert.ok(enabled.has('get_base_schema')); + assert.ok(!enabled.has('delete_table')); + assert.ok(!enabled.has('delete_field')); + assert.ok(!enabled.has('create_table')); + await mgr.switchProfile('full'); + }); }); describe('switchProfile()', () => { From d8fb659c3b9418dc7e5b41ff062383a2eeb19d7c Mon Sep 17 00:00:00 2001 From: "A.R." Date: Fri, 12 Jun 2026 10:03:55 +0300 Subject: [PATCH 06/19] fix(daemon): timing-safe bearer comparison, redact token in daemon status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bearer token comparison used ===, which short-circuits on the first differing byte and is timeable from the tunnel-exposed surface; use crypto.timingSafeEqual. 'daemon status' printed the full bearer token into shell history/scrollback — redact it; the token stays readable from ~/.airtable-user-mcp/daemon.token when genuinely needed. Co-Authored-By: Claude Fable 5 --- packages/mcp-server/src/cli.js | 10 +++++++++- packages/mcp-server/src/daemon/server.js | 13 ++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/packages/mcp-server/src/cli.js b/packages/mcp-server/src/cli.js index 68cdb96..5557aff 100644 --- a/packages/mcp-server/src/cli.js +++ b/packages/mcp-server/src/cli.js @@ -261,7 +261,15 @@ export async function runCli(args) { if (subcmd === 'status') { const { getDaemonStatus } = await import('./daemon/launcher.js'); const status = await getDaemonStatus({ configDir: process.env.AIRTABLE_USER_MCP_HOME }); - process.stdout.write(JSON.stringify(status, null, 2) + '\n'); + // Redact the bearer token — `daemon status` output lands in shell + // history, terminal scrollback, and pasted bug reports. The token is + // readable from ~/.airtable-user-mcp/daemon.token when actually needed. + const redacted = JSON.stringify( + status, + (key, value) => (key === 'bearerToken' && typeof value === 'string' ? '[redacted]' : value), + 2, + ); + process.stdout.write(redacted + '\n'); return true; } process.stderr.write('Unknown daemon subcommand: ' + (subcmd ?? '(none)') + '\n'); diff --git a/packages/mcp-server/src/daemon/server.js b/packages/mcp-server/src/daemon/server.js index 125fd02..29eafc8 100644 --- a/packages/mcp-server/src/daemon/server.js +++ b/packages/mcp-server/src/daemon/server.js @@ -1,5 +1,6 @@ import { createServer } from 'node:http'; import { createRequire } from 'node:module'; +import { timingSafeEqual } from 'node:crypto'; import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import path from 'node:path'; @@ -286,11 +287,21 @@ export async function startDaemonServer(options = {}) { } }; + // Constant-time comparison — `===` short-circuits on the first differing + // byte, which lets a tunnel-side attacker time their way through the token. + const tokensMatch = (provided, expected) => { + if (typeof provided !== 'string' || typeof expected !== 'string') return false; + const a = Buffer.from(provided, 'utf8'); + const b = Buffer.from(expected, 'utf8'); + if (a.length !== b.length) return false; + return timingSafeEqual(a, b); + }; + const requireBearer = (req, res, next) => { const header = req.headers?.authorization ?? ''; const match = header.match(/^Bearer\s+(.+)$/i); const provided = match ? match[1] : null; - if (provided !== currentToken.bearerToken) { + if (!tokensMatch(provided, currentToken.bearerToken)) { track401Burst(req); // 401-burst tripwire (D-06) const wantHtml = (req.headers?.accept ?? '').includes('text/html'); if (wantHtml) { From 1ee41e6f084c8dcb160649dff2f39543ead33e6a Mon Sep 17 00:00:00 2001 From: "A.R." Date: Fri, 12 Jun 2026 10:04:15 +0300 Subject: [PATCH 07/19] docs: changelog entry for security hardening fixes Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d964186..3e7634b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,57 @@ Check [Keep a Changelog](http://keepachangelog.com/) for recommendations on how ## [Unreleased] +### Security hardening — critical-tier fixes from full-codebase audit (2026-06-12) + +A multi-agent security audit (71 findings raised, 49 confirmed under adversarial +verification — full report in `.planning/audits/2026-06-12-hardening-audit.md`) +produced these fixes for the highest-impact tier: + +- **`daemon.lock` no longer world-readable** ([`lockfile.js`](packages/mcp-server/src/daemon/lockfile.js)) — + the lockfile carries the plaintext daemon bearer token but was created with + default permissions. `acquire()` now opens with mode `0o600`, `replace()` + stages its temp file at `0o600` (via `safeAtomicWriteFileSync`), and both + apply the same Windows ACL restriction `daemon.token` already used. The LSP + server's `port_lsp` writer ([`lockfile-writer.ts`](packages/lsp-server/src/lockfile-writer.ts)) + stages at `0o600` too so its atomic rename doesn't undo the hardening. The + shared `applyPrivatePermissions` helper ([`token.js`](packages/mcp-server/src/daemon/token.js)) + is now exported and sanitizes `USERNAME`/`USERDOMAIN` before building the + icacls principal. +- **Login credentials moved off the child environment** ([`auth-manager.ts`](packages/extension/src/mcp/auth-manager.ts), + [`login-runner.js`](packages/mcp-server/src/login-runner.js)) — auto-login + previously passed `AIRTABLE_EMAIL`/`AIRTABLE_PASSWORD`/`AIRTABLE_OTP_SECRET` + via env, visible in `/proc//environ` and core dumps. The runner now + requests credentials over the `fork()` IPC channel + (`request-credentials` → `credentials` handshake) with env retained only as + a standalone-use fallback. The VS Code MCP stdio definition + (`registration.ts`) still uses env — VS Code owns that spawn and env is the + only channel there. +- **Unknown tool profile now fails closed** ([`tool-config.js`](packages/mcp-server/src/tool-config.js)) — + a hand-edited or corrupted `tools-config.json` with an unrecognized + `activeProfile` used to silently enable **all 66 tools** including + destructive ones; it now falls back to the `read-only` set and logs a + warning. `tools-config.json` is also written `0o600`. +- **Release workflow supply-chain pinning** ([`release.yml`](.github/workflows/release.yml)) — + `@vscode/vsce@3.9.2` / `ovsx@1.0.1` installed with exact pins, all + invocations use `npx --no-install` (publish tokens are in scope of those + steps), and Marketplace/npm version replies are validated as strict semver + before being interpolated into version-bump scripts. +- **VSIX packaging symlink guard** ([`prepare-package-deps.mjs`](scripts/prepare-package-deps.mjs)) — + the `dereference: true` copy follows every symlink in the copied packages; + a trojanized dependency could ship a symlink at `~/.ssh` or CI credentials + and have the target land in the published VSIX. The build now walks each + package tree (cycle-safe, through directory-symlink targets) and fails if + any symlink resolves outside the workspace `node_modules` tree. +- **Daemon token hygiene** ([`server.js`](packages/mcp-server/src/daemon/server.js), + [`cli.js`](packages/mcp-server/src/cli.js)) — bearer comparison is now + constant-time (`crypto.timingSafeEqual`), and `airtable-user-mcp daemon + status` redacts the bearer token instead of printing it into shell + history/scrollback. + +Tests: mcp-server 273 (incl. new fail-closed profile test), extension 65, +lsp-server 21 — all pass; `check:tool-sync` green; packaging script verified +against the real pnpm tree; IPC handshake smoke-tested end-to-end. + ### Extension — Windsurf renamed to Devin Desktop, legacy-compatible (2026-06-05) Cognition rebranded the Windsurf editor to **Devin Desktop** on 2026-06-02 (in-place OTA rename) and moved workspace AI assets from `.windsurf/` to `.devin/`, keeping `.windsurf/` as a read fallback (Windsurf-import is on by default). From 5eafc6115e8db43bc0bbb16b35b183e681c31650 Mon Sep 17 00:00:00 2001 From: "A.R." Date: Fri, 12 Jun 2026 10:15:49 +0300 Subject: [PATCH 08/19] fix(build): canonicalize node_modules allowlist root in symlink guard Symlink targets are compared post-realpathSync; an unresolved allowlist entry would reject legitimate pnpm links when the workspace itself sits behind a symlink (e.g. a git worktree). Found by adversarial diff review. Co-Authored-By: Claude Fable 5 --- scripts/prepare-package-deps.mjs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/prepare-package-deps.mjs b/scripts/prepare-package-deps.mjs index 0244795..f41075d 100644 --- a/scripts/prepare-package-deps.mjs +++ b/scripts/prepare-package-deps.mjs @@ -88,7 +88,10 @@ const isWithin = (child, parent) => { }; function assertSafeSymlinks(rootDir) { - const allowedRoots = [realpathSync(rootDir), join(workspaceRoot, 'node_modules')]; + // Both roots must be canonicalized — symlink targets are compared after + // realpathSync, so an un-resolved allowlist entry (workspace itself behind + // a symlink, e.g. a git worktree) would reject legitimate pnpm links. + const allowedRoots = [realpathSync(rootDir), realpathSync(join(workspaceRoot, 'node_modules'))]; const visited = new Set(); const stack = [realpathSync(rootDir)]; From 3a27ffb9313f9e68a958827fc41eb91a8e786765 Mon Sep 17 00:00:00 2001 From: "A.R." Date: Fri, 12 Jun 2026 10:37:27 +0300 Subject: [PATCH 09/19] =?UTF-8?q?fix(daemon):=20make=20dashboard=20Stop=20?= =?UTF-8?q?reliable=20=E2=80=94=20verify=20shutdown,=20reclaim=20stale=20l?= =?UTF-8?q?ocks,=20prevent=20auto-resurrect?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root causes of 'stop button sometimes does not work': 1. DaemonManager.stopDaemon() was fire-and-forget: it ignored the HTTP response status (a 401 from a stale lockfile token looked like success), swallowed connection errors (dead daemon with a leftover lock), and returned before the daemon released its lockfile — so the immediate pushState() re-read the still-present lock and the UI kept showing 'running'. It now mirrors the CLI launcher semantics: verify the shutdown response, wait for lock release, escalate SIGTERM → SIGKILL when the daemon answers but will not exit, and reclaim stale lockfiles (without killing when the port is unreachable — the pid may have been recycled onto an innocent process). 2. With useDaemon + loginMode=auto, provideMcpServerDefinitions → getCredentials() → ensureDaemon() respawned the daemon right after a stop (VS Code re-queries definitions when the daemon connection drops). A user-stopped latch now blocks implicit respawns; explicit start/restart clears it. The implicit ensure is also best-effort so daemon startup failures no longer block login. 3. Stop failures are now surfaced in the UI and the stopDaemon command instead of silently reporting success. Lockfile port is also range-validated (audit finding). 6 new tests cover graceful stop, 401 escalation, stale-lock reclaim, PID-reuse safety, and the resurrect latch. Co-Authored-By: Claude Fable 5 --- packages/extension/src/extension.ts | 8 +- packages/extension/src/mcp/auth-manager.ts | 10 +- packages/extension/src/mcp/daemon-manager.ts | 150 +++++++++++++++--- .../extension/src/test/daemon-manager.test.ts | 130 ++++++++++++++- .../src/webview/DashboardProvider.ts | 14 +- 5 files changed, 287 insertions(+), 25 deletions(-) diff --git a/packages/extension/src/extension.ts b/packages/extension/src/extension.ts index a469994..e90b0d8 100644 --- a/packages/extension/src/extension.ts +++ b/packages/extension/src/extension.ts @@ -562,8 +562,12 @@ export async function activate(context: vscode.ExtensionContext): Promise await toolProfileManager.openConfigFile(); }), vscode.commands.registerCommand('airtable-formula.stopDaemon', async () => { - await daemonManager.stopDaemon(); - vscode.window.showInformationMessage('Airtable Formula: Daemon stopped.'); + const result = await daemonManager.stopDaemon(); + if (result.stopped) { + vscode.window.showInformationMessage(`Airtable Formula: Daemon stopped.${result.reason ? ` (${result.reason})` : ''}`); + } else { + vscode.window.showErrorMessage(`Airtable Formula: Daemon stop failed — ${result.reason ?? 'daemon did not exit'}.`); + } dashboardProvider.refresh(); }), vscode.commands.registerCommand('airtable-formula.restartDaemon', async () => { diff --git a/packages/extension/src/mcp/auth-manager.ts b/packages/extension/src/mcp/auth-manager.ts index 0ef6482..9a4243f 100644 --- a/packages/extension/src/mcp/auth-manager.ts +++ b/packages/extension/src/mcp/auth-manager.ts @@ -242,9 +242,15 @@ export class AuthManager implements vscode.Disposable { * Returns undefined if no credentials are stored. */ async getCredentials(): Promise<{ email: string; password: string; otpSecret?: string } | undefined> { - // D-02: ensure daemon is running before handing off credentials + // D-02: ensure daemon is running before handing off credentials. + // Implicit + best-effort: credentials don't require the daemon, and this + // path runs from provideMcpServerDefinitions — it must neither resurrect + // a daemon the user explicitly stopped nor block login when the daemon + // can't start. if (getSettings().mcp.useDaemon && this._daemonManager) { - await this._daemonManager.ensureDaemon(); + try { + await this._daemonManager.ensureDaemon({ implicit: true }); + } catch { /* user-stopped latch or startup failure — proceed without daemon */ } } const email = await this.getEmail(); const password = await this.getPassword(); diff --git a/packages/extension/src/mcp/daemon-manager.ts b/packages/extension/src/mcp/daemon-manager.ts index 03140d0..e8b6f41 100644 --- a/packages/extension/src/mcp/daemon-manager.ts +++ b/packages/extension/src/mcp/daemon-manager.ts @@ -1,6 +1,7 @@ import * as vscode from 'vscode'; import * as path from 'path'; import * as fs from 'fs/promises'; +import { existsSync, rmSync } from 'fs'; import { spawn } from 'child_process'; export interface DaemonStatus { @@ -29,12 +30,25 @@ const EMPTY_STATUS: DaemonStatus = { port_lsp: null, bearerToken: null, tunnelUrl: null, uptime: null, }; +export interface StopResult { + stopped: boolean; + forced: boolean; + reason?: string; +} + export class DaemonManager implements vscode.Disposable { private readonly _onDidChange = new vscode.EventEmitter(); public readonly onDidChange = this._onDidChange.event; private _disposed = false; private _status: DaemonStatus = { ...EMPTY_STATUS }; + /** + * Set when the user explicitly stops the daemon. Implicit ensureDaemon() + * callers (MCP definition provider, credential handoff) respect it so the + * daemon doesn't resurrect seconds after the user pressed Stop. Cleared by + * any explicit start/restart. + */ + private _userStopped = false; constructor( private readonly configDir: string, @@ -62,7 +76,8 @@ export class DaemonManager implements vscode.Disposable { const lockPath = path.join(this.configDir, 'daemon.lock'); const raw = await fs.readFile(lockPath, 'utf8'); const record = JSON.parse(raw) as Record; - const port = typeof record.port === 'number' ? record.port : null; + const port = typeof record.port === 'number' && Number.isInteger(record.port) + && record.port >= 1 && record.port <= 65535 ? record.port : null; const bearerToken = typeof record.bearerToken === 'string' ? record.bearerToken : null; const healthy = port != null && bearerToken != null ? await this._httpHealthCheck(port, bearerToken) @@ -100,8 +115,9 @@ export class DaemonManager implements vscode.Disposable { child.unref(); } - async ensureDaemon(options?: { timeoutMs?: number }): Promise { + async ensureDaemon(options?: { timeoutMs?: number; implicit?: boolean }): Promise { if (this._disposed) throw new Error('DaemonManager disposed'); + if (!options?.implicit) this._userStopped = false; const deadline = Date.now() + (options?.timeoutMs ?? 15_000); let spawned = false; while (Date.now() < deadline) { @@ -118,38 +134,136 @@ export class DaemonManager implements vscode.Disposable { }; } if (!status.running && !spawned) { + if (options?.implicit && this._userStopped) { + throw new Error('Daemon was explicitly stopped by the user; not respawning implicitly.'); + } await this._spawnDetached(); spawned = true; } - await new Promise(resolve => setTimeout(resolve, 200)); + await this._delay(200); } throw new Error('Timed out waiting for daemon startup.'); } - async stopDaemon(): Promise { + /** + * Stop the daemon and only report success once it is actually gone. + * + * Mirrors the CLI launcher's stopDaemon semantics: graceful HTTP shutdown + * (verifying the response — a 401 from a stale lockfile token is a + * failure, not a success), wait for the daemon to release its lockfile, + * escalate to killing the recorded pid when the daemon answers but won't + * exit, and reclaim stale lockfiles so the UI can't get stuck showing a + * dead daemon as "running". + */ + async stopDaemon(): Promise { + this._userStopped = true; const status = await this.getDaemonStatus(); - if (!status.running || status.port == null || status.bearerToken == null) return; - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), 3_000); - try { - await fetch(`http://127.0.0.1:${status.port}/daemon/shutdown`, { - method: 'POST', - headers: { Authorization: `Bearer ${status.bearerToken}` }, - signal: controller.signal, - }); - } catch { - // daemon terminates itself; ignore network errors - } finally { - clearTimeout(timeout); + if (!status.running) return { stopped: true, forced: false }; + + // 1) Graceful shutdown request. Distinguish: accepted / rejected (the + // port answered with an error status) / unreachable (no daemon there). + let outcome: 'accepted' | 'rejected' | 'unreachable' = 'unreachable'; + if (status.port != null && status.bearerToken != null) { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 3_000); + try { + const res = await fetch(`http://127.0.0.1:${status.port}/daemon/shutdown`, { + method: 'POST', + headers: { Authorization: `Bearer ${status.bearerToken}` }, + signal: controller.signal, + }); + outcome = res.ok ? 'accepted' : 'rejected'; + } catch { + outcome = 'unreachable'; + } finally { + clearTimeout(timeout); + } + } + + // 2) Accepted — the daemon releases its lockfile as it exits; wait for it. + if (outcome === 'accepted') { + const deadline = Date.now() + 10_000; + while (Date.now() < deadline) { + if (!this._lockfileExists()) return { stopped: true, forced: false }; + await this._delay(200); + } + // Daemon acknowledged but never exited — fall through to escalation. + } + + const pid = status.pid; + const pidAlive = typeof pid === 'number' && pid > 0 && this._isPidAlive(pid); + + // 3) The port answered, so the pid in the lockfile really is the daemon — + // safe to kill. When the port is unreachable we do NOT kill: after a + // crash the OS may have recycled the pid onto an innocent process. + if (outcome !== 'unreachable' && pidAlive && typeof pid === 'number') { + this._killPid(pid); + const deadline = Date.now() + 3_000; + let escalated = false; + while (Date.now() < deadline && this._isPidAlive(pid)) { + if (!escalated && Date.now() > deadline - 1_500) { + this._killPid(pid, 'SIGKILL'); + escalated = true; + } + await this._delay(100); + } + if (this._isPidAlive(pid)) { + return { stopped: false, forced: true, reason: `Daemon process ${pid} did not exit after kill.` }; + } + this._reclaimLockfile(); + return { stopped: true, forced: true }; } + + // 4) Unreachable: the lockfile is stale (dead pid) or points at a process + // that is not serving the daemon port. Reclaim the lock either way so + // the dashboard stops showing a phantom daemon. + this._reclaimLockfile(); + return { + stopped: true, + forced: false, + reason: pidAlive + ? `Removed stale daemon.lock; process ${pid} was not answering on the daemon port and was left untouched.` + : undefined, + }; } async restartDaemon(): Promise { await this.stopDaemon(); - await new Promise(resolve => setTimeout(resolve, 500)); + await this._delay(500); return this.ensureDaemon(); } + // ─── Stop helpers (instance methods so tests can stub process control) ── + + private _lockfileExists(): boolean { + return existsSync(path.join(this.configDir, 'daemon.lock')); + } + + private _reclaimLockfile(): void { + try { + rmSync(path.join(this.configDir, 'daemon.lock'), { force: true }); + } catch { /* best-effort */ } + } + + private _isPidAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (err) { + return (err as NodeJS.ErrnoException)?.code === 'EPERM'; + } + } + + private _killPid(pid: number, signal: NodeJS.Signals = 'SIGTERM'): void { + try { + process.kill(pid, signal); + } catch { /* already gone */ } + } + + private _delay(ms: number): Promise { + return new Promise(resolve => setTimeout(resolve, ms)); + } + buildDaemonEnv(credEnv?: Record): Record { const env: Record = { AIRTABLE_USER_MCP_HOME: this.configDir, diff --git a/packages/extension/src/test/daemon-manager.test.ts b/packages/extension/src/test/daemon-manager.test.ts index cd99ecc..941a3ed 100644 --- a/packages/extension/src/test/daemon-manager.test.ts +++ b/packages/extension/src/test/daemon-manager.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import * as os from 'os'; import * as path from 'path'; import * as fs from 'fs'; @@ -79,6 +79,134 @@ describe('DaemonManager.probeHealth', () => { }); }); +describe('DaemonManager.stopDaemon', () => { + let tmpDir: string; + let dm: InstanceType; + let server: import('http').Server | undefined; + + const writeLock = (port: number, pid = process.pid, token = 'test-token') => { + fs.writeFileSync(path.join(tmpDir, 'daemon.lock'), JSON.stringify({ + pid, uuid: 'uuid-1', port, port_lsp: null, bearerToken: token, + version: '0.0.0', startedAt: new Date().toISOString(), tunnelUrl: null, + })); + }; + const lockExists = () => fs.existsSync(path.join(tmpDir, 'daemon.lock')); + + const listen = async (handler: import('http').RequestListener): Promise => { + const http = await import('http'); + server = http.createServer(handler); + await new Promise(r => server!.listen(0, '127.0.0.1', r)); + return (server!.address() as import('net').AddressInfo).port; + }; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'test-daemon-stop-')); + dm = new DaemonManager(tmpDir, '/tmp/test-ext-path'); + }); + + afterEach(async () => { + if (server) { await new Promise(r => server!.close(() => r())); server = undefined; } + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('returns stopped:true immediately when no lockfile exists', async () => { + const result = await dm.stopDaemon(); + expect(result.stopped).toBe(true); + expect(result.forced).toBe(false); + }); + + it('graceful: waits for the daemon to release its lockfile before resolving', async () => { + const port = await listen((req, res) => { + if (req.method === 'POST' && req.url === '/daemon/shutdown') { + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end('{"ok":true}'); + // Simulate the daemon releasing the lock shortly after replying + setTimeout(() => fs.rmSync(path.join(tmpDir, 'daemon.lock'), { force: true }), 150); + return; + } + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end('{"ok":true}'); + }); + writeLock(port); + + const result = await dm.stopDaemon(); + expect(result.stopped).toBe(true); + expect(result.forced).toBe(false); + expect(lockExists()).toBe(false); + }); + + it('rejected shutdown (401): escalates to killing the recorded pid and reclaims the lock', async () => { + const port = await listen((_req, res) => { + res.writeHead(401, { 'Content-Type': 'application/json' }); + res.end('{"error":"Unauthorized"}'); + }); + writeLock(port, 12345, 'stale-token'); + + let killed = false; + (dm as any)._killPid = vi.fn(() => { killed = true; }); + (dm as any)._isPidAlive = vi.fn(() => !killed); + + const result = await dm.stopDaemon(); + expect((dm as any)._killPid).toHaveBeenCalledWith(12345); + expect(result.stopped).toBe(true); + expect(result.forced).toBe(true); + expect(lockExists()).toBe(false); + }); + + it('unreachable daemon with dead pid: reclaims the stale lock without killing anything', async () => { + writeLock(1, 999_999); // port 1 — nothing listening + (dm as any)._killPid = vi.fn(); + (dm as any)._isPidAlive = vi.fn(() => false); + + const result = await dm.stopDaemon(); + expect((dm as any)._killPid).not.toHaveBeenCalled(); + expect(result.stopped).toBe(true); + expect(lockExists()).toBe(false); + }); + + it('unreachable daemon with live pid: does NOT kill (PID-reuse safety) but reclaims the lock', async () => { + writeLock(1, process.pid); + (dm as any)._killPid = vi.fn(); + (dm as any)._isPidAlive = vi.fn(() => true); + + const result = await dm.stopDaemon(); + expect((dm as any)._killPid).not.toHaveBeenCalled(); + expect(result.stopped).toBe(true); + expect(result.reason).toBeTruthy(); + expect(lockExists()).toBe(false); + }); +}); + +describe('DaemonManager user-stopped latch', () => { + let tmpDir: string; + let dm: InstanceType; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'test-daemon-latch-')); + dm = new DaemonManager(tmpDir, '/tmp/test-ext-path'); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('implicit ensureDaemon refuses to respawn after an explicit stop', async () => { + (dm as any)._spawnDetached = vi.fn(); + await dm.stopDaemon(); // no lock — still sets the latch + await expect(dm.ensureDaemon({ implicit: true, timeoutMs: 500 })).rejects.toThrow(/stopped/i); + expect((dm as any)._spawnDetached).not.toHaveBeenCalled(); + }); + + it('explicit ensureDaemon clears the latch and attempts a spawn', async () => { + (dm as any)._spawnDetached = vi.fn(); + await dm.stopDaemon(); + await expect(dm.ensureDaemon({ timeoutMs: 400 })).rejects.toThrow(/Timed out/); + expect((dm as any)._spawnDetached).toHaveBeenCalled(); + // Latch cleared — implicit calls may spawn again now + await expect(dm.ensureDaemon({ implicit: true, timeoutMs: 400 })).rejects.toThrow(/Timed out/); + }); +}); + describe('createHttpDefinition', () => { it('returns null when McpHttpServerDefinition is not on vscode namespace', () => { // vscode mock does not include McpHttpServerDefinition — expect null diff --git a/packages/extension/src/webview/DashboardProvider.ts b/packages/extension/src/webview/DashboardProvider.ts index 815928d..7decae3 100644 --- a/packages/extension/src/webview/DashboardProvider.ts +++ b/packages/extension/src/webview/DashboardProvider.ts @@ -535,9 +535,19 @@ export class DashboardProvider implements vscode.WebviewViewProvider { if (msg.type === 'daemon:stop') { try { - await this._daemonManager?.stopDaemon(); + const result = await this._daemonManager?.stopDaemon(); await this.pushState(); - this.postResult(msg.id, true); + if (result && !result.stopped) { + const reason = result.reason ?? 'Daemon did not exit.'; + vscode.window.showErrorMessage(`Daemon stop failed: ${reason}`); + this.postResult(msg.id, false, reason); + } else { + if (result?.reason) { + // Stopped, but with a caveat (e.g. stale lock cleaned up) — inform, don't alarm. + vscode.window.showInformationMessage(`Daemon stopped: ${result.reason}`); + } + this.postResult(msg.id, true); + } } catch (err) { vscode.window.showErrorMessage(`Daemon stop failed: ${err instanceof Error ? err.message : String(err)}`); this.postResult(msg.id, false, String(err)); From 26a81563afbfbbed6a9194b2bd217a340e904d12 Mon Sep 17 00:00:00 2001 From: "A.R." Date: Fri, 12 Jun 2026 10:48:13 +0300 Subject: [PATCH 10/19] =?UTF-8?q?fix(webview):=20UI/UX=20hardening=20?= =?UTF-8?q?=E2=80=94=20action=20feedback,=20state=20sync,=20theming,=20a11?= =?UTF-8?q?y,=20content?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 26 confirmed findings from a multi-agent UI/UX audit of the dashboard: - store.ts: all async actions track pending state (refresh, selectCustomBrowser, setBrowserChoice, copyAirtablePat, openStoragePath were missing it); pending actions auto-expire (60s default, longer for login/browser downloads) so a lost action:result can no longer leave every button disabled forever. - DashboardProvider: pushState() serialized + coalesced (file-watch bursts could post a stale state:update last); re-sync on onDidChangeVisibility; daemon:start posts failure when the daemon manager is unavailable; toolProfile fallback includes all 13 categories (recordRead/recordWrite were missing). - Theming: undefined --accent-green replaced with --fg-ok; LSP badges and error/warn banners use theme tokens; new --border-error, --border-warn, --bg-lsp-* tokens. - A11y: global :focus-visible outlines for buttons/inputs/selects; --fg-muted contrast raised 2.86:1 -> ~4.8:1 (WCAG AA); login toggle gets role=switch + aria-label; daemon buttons get aria-busy; disabled buttons get not-allowed cursor. - Content: friendlyError.ts maps raw errno/HTTP errors to readable guidance (raw kept in tooltip); TOTP/bearer jargon explained; daemon tunnel URL row made responsive with Copy button; IDE detection shows animated skeleton; dropped webview messages are logged. Co-Authored-By: Claude Fable 5 --- .../src/webview/DashboardProvider.ts | 43 +++++++-- packages/webview/src/components/IdeCard.tsx | 4 +- packages/webview/src/lib/friendlyError.ts | 72 +++++++++++++++ packages/webview/src/lib/vscode.ts | 12 ++- packages/webview/src/store.ts | 91 ++++++++++++------- packages/webview/src/styles.css | 40 +++++++- packages/webview/src/tabs/Overview.tsx | 2 +- packages/webview/src/tabs/Settings.tsx | 51 ++++++++--- packages/webview/src/tabs/Setup.tsx | 47 +++++++--- 9 files changed, 293 insertions(+), 69 deletions(-) create mode 100644 packages/webview/src/lib/friendlyError.ts diff --git a/packages/extension/src/webview/DashboardProvider.ts b/packages/extension/src/webview/DashboardProvider.ts index 7decae3..110c426 100644 --- a/packages/extension/src/webview/DashboardProvider.ts +++ b/packages/extension/src/webview/DashboardProvider.ts @@ -71,6 +71,11 @@ export class DashboardProvider implements vscode.WebviewViewProvider { }; webviewView.webview.html = getWebviewHtml(webviewView.webview, this.context); webviewView.webview.onDidReceiveMessage(msg => this.handleMessage(msg as WebviewMessage)); + // Re-sync when the sidebar is re-opened — daemon/tunnel/auth state may + // have changed while the view was hidden and no watcher fired since. + webviewView.onDidChangeVisibility(() => { + if (webviewView.visible) void this.pushState(); + }); } private async handleMessage(msg: WebviewMessage): Promise { @@ -516,9 +521,9 @@ export class DashboardProvider implements vscode.WebviewViewProvider { // feedback, then run ensureDaemon in the background (can take ~15s). this._daemonStarting = true; void this.pushState(); - this.postResult(msg.id, true); const dm = this._daemonManager; if (dm) { + this.postResult(msg.id, true); dm.restartDaemon() .then(() => { this._daemonStarting = false; void this._initLockfileWatch(); return this.pushState(); }) .catch(err => { @@ -529,6 +534,7 @@ export class DashboardProvider implements vscode.WebviewViewProvider { } else { this._daemonStarting = false; void this.pushState(); + this.postResult(msg.id, false, 'Daemon manager unavailable'); } return; } @@ -711,7 +717,32 @@ export class DashboardProvider implements vscode.WebviewViewProvider { } } - async pushState(): Promise { + // pushState is async and reads daemon.lock / settings from disk; concurrent + // runs (e.g. several fs.watch events in a burst) can finish out of order and + // post a STALE state:update last. Serialize: one run at a time, and coalesce + // requests that arrive mid-run into a single trailing re-run. + private _pushInFlight: Promise | null = null; + private _pushQueued = false; + + pushState(): Promise { + if (this._pushInFlight) { + this._pushQueued = true; + return this._pushInFlight; + } + this._pushInFlight = (async () => { + try { + do { + this._pushQueued = false; + await this._computeAndPostState(); + } while (this._pushQueued); + } finally { + this._pushInFlight = null; + } + })(); + return this._pushInFlight; + } + + private async _computeAndPostState(): Promise { if (!this.view) return; this._debugCollector?.trace('ext', 'webview', 'webview:message_out', { type: 'state:update', @@ -741,15 +772,15 @@ export class DashboardProvider implements vscode.WebviewViewProvider { // during very early activation. const toolProfile: ToolProfileSnapshot = this.toolProfileManager?.getSnapshot() ?? { profile: 'full', - enabledCount: 62, - totalCount: 62, + enabledCount: 66, + totalCount: 66, categories: { - read: true, + read: true, recordRead: true, tableWrite: true, tableDestructive: true, fieldWrite: true, fieldDestructive: true, viewWrite: true, viewDestructive: true, viewSection: true, viewSectionDestructive: true, - formWrite: true, + formWrite: true, recordWrite: true, extension: true, }, }; diff --git a/packages/webview/src/components/IdeCard.tsx b/packages/webview/src/components/IdeCard.tsx index 2573169..6a1dc9b 100644 --- a/packages/webview/src/components/IdeCard.tsx +++ b/packages/webview/src/components/IdeCard.tsx @@ -35,8 +35,8 @@ function LspBadge({ active = true }: { active?: boolean }) { display: 'inline-flex', alignItems: 'center', justifyContent: 'center', fontSize: '0.58rem', fontWeight: 700, fontFamily: 'var(--font-mono)', color: active ? 'var(--fg-ok)' : 'var(--fg-err)', - background: active ? 'rgba(34,197,94,0.12)' : 'rgba(239,68,68,0.1)', - border: `1px solid ${active ? 'rgba(34,197,94,0.3)' : 'rgba(239,68,68,0.3)'}`, + background: active ? 'var(--bg-lsp-ok)' : 'var(--bg-lsp-err)', + border: `1px solid ${active ? 'var(--border-lsp-ok)' : 'var(--border-lsp-err)'}`, borderRadius: 3, padding: '0 4px', lineHeight: '16px', flexShrink: 0, }}>LSP ); diff --git a/packages/webview/src/lib/friendlyError.ts b/packages/webview/src/lib/friendlyError.ts new file mode 100644 index 0000000..b25dcfe --- /dev/null +++ b/packages/webview/src/lib/friendlyError.ts @@ -0,0 +1,72 @@ +/** + * Map raw error strings (Node errno codes, HTTP statuses, patchright + * internals) to human-readable guidance. The raw text stays available for + * tooltips / bug reports; users see what happened and what to do next. + */ +export interface FriendlyError { + message: string; + hint?: string; + raw: string; +} + +const PATTERNS: Array<{ test: RegExp; message: string; hint?: string }> = [ + { + test: /ENOTFOUND|EAI_AGAIN|ECONNREFUSED|ECONNRESET|ETIMEDOUT|fetch failed|network/i, + message: 'Network error while contacting the service.', + hint: 'Check your internet connection (or proxy/VPN) and try again.', + }, + { + test: /401|Unauthorized/i, + message: 'Authentication was rejected.', + hint: 'Your Airtable session may have expired — try logging in again.', + }, + { + test: /403|Forbidden/i, + message: 'Access denied by Airtable.', + hint: 'Your account may lack permission for this base or action.', + }, + { + test: /429|rate.?limit/i, + message: 'Airtable is rate-limiting requests.', + hint: 'Wait a minute and try again.', + }, + { + test: /exit code 21|launchPersistentContext|Target page, context or browser has been closed/i, + message: 'The browser could not start (its profile may be locked).', + hint: 'Close any leftover Chrome windows from a previous login and retry.', + }, + { + test: /No supported browser|executable doesn't exist|chrome-missing/i, + message: 'No usable browser was found.', + hint: 'Install Google Chrome, or use "Download bundled Chromium" below.', + }, + { + test: /ENOENT/i, + message: 'A required file or folder is missing.', + hint: 'Try running Setup again to recreate the configuration.', + }, + { + test: /EACCES|EPERM/i, + message: 'Permission denied while accessing a file.', + hint: 'Another program may be locking it, or it needs elevated rights.', + }, + { + test: /ENOSPC/i, + message: 'The disk is full.', + hint: 'Free up disk space and retry.', + }, + { + test: /timed? ?out/i, + message: 'The operation timed out.', + hint: 'The service may be slow right now — try again.', + }, +]; + +export function friendlyError(raw: string | null | undefined): FriendlyError | null { + if (!raw) return null; + for (const p of PATTERNS) { + if (p.test.test(raw)) return { message: p.message, hint: p.hint, raw }; + } + // Unrecognized — show the raw text but keep it as the message so nothing is hidden. + return { message: raw, raw }; +} diff --git a/packages/webview/src/lib/vscode.ts b/packages/webview/src/lib/vscode.ts index d329bdd..8c75651 100644 --- a/packages/webview/src/lib/vscode.ts +++ b/packages/webview/src/lib/vscode.ts @@ -8,11 +8,19 @@ declare function acquireVsCodeApi(): { const vscodeApi = (() => { try { return acquireVsCodeApi(); } - catch { return null; } + catch { + // Expected in the browser dev preview (vite dev); fatal inside VS Code. + console.warn('[webview] acquireVsCodeApi unavailable — messages to the extension will be dropped'); + return null; + } })(); export function sendToExtension(msg: WebviewMessage): void { - vscodeApi?.postMessage(msg); + if (!vscodeApi) { + console.warn('[webview] dropped message (no VS Code API):', msg.type); + return; + } + vscodeApi.postMessage(msg); } export function onExtensionMessage(handler: (msg: ExtensionMessage) => void): () => void { diff --git a/packages/webview/src/store.ts b/packages/webview/src/store.ts index 987732f..d6b15c5 100644 --- a/packages/webview/src/store.ts +++ b/packages/webview/src/store.ts @@ -77,7 +77,22 @@ const defaultAuth: AuthState = { hasCredentials: false, }; -export const useStore = create((set, get) => ({ +// If the extension never answers (host crash, lost message, webview reload), +// pending actions must not keep buttons disabled forever — auto-expire them. +const PENDING_TIMEOUT_MS = 60_000; +const pendingTimers = new Map>(); + +export const useStore = create((set, get) => { + /** Track an in-flight action and schedule its auto-expiry. */ + const beginAction = (id: string, timeoutMs = PENDING_TIMEOUT_MS) => { + set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + pendingTimers.set(id, setTimeout(() => { + pendingTimers.delete(id); + get().markActionDone(id, false); + }, timeoutMs)); + }; + + return ({ ideStatuses: [], versions: { extension: '—', mcpServerBundled: '—' }, aiFilesCount: 0, @@ -111,157 +126,165 @@ export const useStore = create((set, get) => ({ setupIde: (ideId) => { const id = randomId(); + beginAction(id); set(s => { - const nextPending = new Set([...s.pendingActions, id]); const nextIde = new Map(s.pendingIdeActions); nextIde.set(ideId, id); - return { pendingActions: nextPending, pendingIdeActions: nextIde }; + return { pendingIdeActions: nextIde }; }); sendToExtension({ type: 'action:setupIde', id, ideId: ideId as any }); }, setupAll: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:setupAll', id }); }, refresh: () => { const id = randomId(); + beginAction(id); sendToExtension({ type: 'action:refresh', id }); }, login: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + // Auto-login drives a real browser — can legitimately take minutes. + beginAction(id, 360_000); sendToExtension({ type: 'action:login', id }); }, logout: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:logout', id }); }, status: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:status', id }); }, saveCredentials: (email, password, otpSecret) => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:saveCredentials', id, email, password, otpSecret }); }, installBrowser: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + // Chromium download can take minutes on slow connections. + beginAction(id, 600_000); sendToExtension({ type: 'action:install-browser', id }); }, removeBrowser: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:removeBrowser', id }); }, unconfigureIde: (ideId) => { const id = randomId(); + beginAction(id); set(s => { - const nextPending = new Set([...s.pendingActions, id]); const nextIde = new Map(s.pendingIdeActions); nextIde.set(ideId, id); - return { pendingActions: nextPending, pendingIdeActions: nextIde }; + return { pendingIdeActions: nextIde }; }); sendToExtension({ type: 'action:unconfigureIde', id, ideId: ideId as any }); }, debugStartSession: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:debug.startSession', id }); }, debugStopAndExport: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:debug.stopAndExport', id }); }, debugExport: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:debug.export', id }); }, manualLogin: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + // Manual login waits for the user to finish in the browser (up to ~5.5m). + beginAction(id, 360_000); sendToExtension({ type: 'action:manualLogin', id }); }, backupSession: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:backupSession', id }); }, restoreSession: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:restoreSession', id }); }, selectCustomBrowser: () => { const id = randomId(); + // Long timeout — the native file dialog stays open until the user acts. + beginAction(id, 600_000); sendToExtension({ type: 'action:selectCustomBrowser', id }); }, setBrowserChoice: (choice) => { const id = randomId(); + beginAction(id); sendToExtension({ type: 'action:setBrowserChoice', id, choice }); }, openStoragePath: (p) => { const id = randomId(); + beginAction(id); sendToExtension({ type: 'action:openStoragePath', id, path: p }); }, enableTunnel: (provider, authtoken, domain) => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'tunnel:enable', id, provider, authtoken, domain }); }, disableTunnel: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'tunnel:disable', id }); }, setNgrokAuthtoken: (authtoken) => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'tunnel:set-ngrok-authtoken', id, authtoken }); }, startDaemon: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'daemon:start', id }); }, stopDaemon: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'daemon:stop', id }); }, restartDaemon: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'daemon:restart', id }); }, @@ -272,52 +295,55 @@ export const useStore = create((set, get) => ({ rotateToken: () => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'daemon:rotate-token', id }); }, saveAirtablePat: (pat) => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:save-airtable-pat', id, pat }); }, copyAirtablePat: () => { const id = randomId(); + beginAction(id); sendToExtension({ type: 'action:copy-airtable-pat', id }); }, configureOfficialAirtable: (ideId) => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:configure-official-airtable', id, ideId }); }, unconfigureOfficialAirtable: (ideId) => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:unconfigure-official-airtable', id, ideId }); }, savePrompt: (prompt) => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:save-prompt', id, prompt }); }, deletePrompt: (name) => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:delete-prompt', id, name }); }, resetPrompt: (name) => { const id = randomId(); - set(s => ({ pendingActions: new Set([...s.pendingActions, id]) })); + beginAction(id); sendToExtension({ type: 'action:reset-prompt', id, name }); }, markActionDone: (id, _ok) => { + const timer = pendingTimers.get(id); + if (timer) { clearTimeout(timer); pendingTimers.delete(id); } set(s => { const next = new Set(s.pendingActions); next.delete(id); @@ -328,4 +354,5 @@ export const useStore = create((set, get) => ({ return { pendingActions: next, pendingIdeActions: nextIde }; }); }, -})); + }); +}); diff --git a/packages/webview/src/styles.css b/packages/webview/src/styles.css index 072a216..5b15867 100644 --- a/packages/webview/src/styles.css +++ b/packages/webview/src/styles.css @@ -7,6 +7,9 @@ --at-gray700: rgb(49,53,62); --at-gray600: rgb(65,69,77); --at-gray500: rgb(97,102,112); + /* gray450: between gray500/gray400 — gray500 on --bg is 2.86:1, failing + WCAG AA (4.5:1) for the small muted text it backs; this hits ~4.8:1. */ + --at-gray450: rgb(133,139,150); --at-gray400: rgb(151,154,160); --at-blue: rgb(22,110,225); --at-blueLight1: rgb(160,198,255); @@ -35,10 +38,18 @@ --border: rgba(255,255,255,0.10); --border-em: rgba(255,255,255,0.25); --border-focus: var(--at-blue); + --border-error: rgba(220,4,59,0.25); + --border-warn: rgba(255,186,5,0.25); + + /* LSP status badge fills (IdeCard) — derived from --at-green / --at-red */ + --bg-lsp-ok: rgba(34,197,94,0.12); + --bg-lsp-err: rgba(239,68,68,0.10); + --border-lsp-ok: rgba(34,197,94,0.3); + --border-lsp-err: rgba(239,68,68,0.25); --fg: #ffffff; --fg-subtle: var(--at-gray400); - --fg-muted: var(--at-gray500); + --fg-muted: var(--at-gray450); --fg-ai: var(--at-pinkLight1); --fg-info: var(--at-blueLight1); --fg-ok: var(--at-greenLight1); @@ -65,9 +76,36 @@ body { -webkit-font-smoothing: antialiased; } button { font: inherit; cursor: pointer; border: none; background: none; } +/* Keyboard focus must be visible on every interactive control, including + icon-only buttons that carry no .btn/.action-card class. */ +button:focus-visible, +.input-field:focus-visible, +select:focus-visible { + outline: 2px solid var(--border-focus); + outline-offset: 2px; +} +button:disabled, +button[aria-disabled="true"] { + opacity: 0.5; + cursor: not-allowed; +} +.input-field:focus-visible { border-color: var(--at-blue); } /* ─── Design System Classes ─── */ +/* Skeleton loading rows (Setup tab while IDE detection runs) */ +.skeleton-row { + height: 38px; + border-radius: 10px; + background: linear-gradient(90deg, rgba(255,255,255,0.04) 25%, rgba(255,255,255,0.09) 50%, rgba(255,255,255,0.04) 75%); + background-size: 200% 100%; + animation: skeleton-shimmer 1.4s ease-in-out infinite; +} +@keyframes skeleton-shimmer { + 0% { background-position: 200% 0; } + 100% { background-position: -200% 0; } +} + /* Glass Panel */ .glass-panel { border: 1px solid var(--border); diff --git a/packages/webview/src/tabs/Overview.tsx b/packages/webview/src/tabs/Overview.tsx index f374c23..71e8861 100644 --- a/packages/webview/src/tabs/Overview.tsx +++ b/packages/webview/src/tabs/Overview.tsx @@ -79,7 +79,7 @@ export function Overview() { Extension {versions.extension} MCP {versions.mcpServerBundled} bundled {versions.mcpServerPublished && ( - ↑ update: {versions.mcpServerPublished} + ↑ update: {versions.mcpServerPublished} )} diff --git a/packages/webview/src/tabs/Settings.tsx b/packages/webview/src/tabs/Settings.tsx index 0720604..bf753f4 100644 --- a/packages/webview/src/tabs/Settings.tsx +++ b/packages/webview/src/tabs/Settings.tsx @@ -1,6 +1,7 @@ import React, { useState, useEffect, useCallback } from 'react'; import { useStore } from '../store.js'; import { sendToExtension } from '../lib/vscode.js'; +import { friendlyError } from '../lib/friendlyError.js'; import { StatusDot } from '../components/StatusDot.js'; import { LogIn, LogOut, RefreshCw, Shield, Key, Clock, Globe, AlertTriangle, Download, Trash2, Sliders, FileJson, FolderOpen, ChevronDown, ChevronRight, Archive, Upload } from 'lucide-react'; @@ -131,7 +132,14 @@ export function Settings() {
Manual Auto @@ -216,7 +224,7 @@ export function Settings() { )} {chromeMissing && ( -
+
@@ -261,11 +269,17 @@ export function Settings() {
)} - {downloadError && ( -
- Download failed: {dl?.error} -
- )} + {downloadError && (() => { + const fe = friendlyError(dl?.error); + return ( +
+ Download failed: {fe?.message} + {fe?.hint && ( + {fe.hint} + )} +
+ ); + })()}
)} @@ -306,11 +320,16 @@ export function Settings() { setOtpSecret(e.target.value)} style={{ fontSize: '0.75rem', padding: '6px 10px', borderRadius: 8, border: '1px solid var(--border)', background: 'var(--bg-input)', color: 'var(--fg)' }} /> +
+ 2FA secret: the base32 code Airtable shows when you set up an authenticator + app (also called a TOTP secret). Leave empty if 2FA is disabled. +
)} @@ -408,15 +426,15 @@ export function Setup() { {/* Daemon controls */}
{!daemon?.running ? ( - ) : ( <> - - @@ -679,8 +697,13 @@ export function Setup() { )} {ideStatuses.length === 0 && ( -
- No IDE data available yet. Loading... +
+ {[0, 1, 2].map(i => ( +
+ ))} +
+ Scanning for installed IDEs… +
)} @@ -767,7 +790,7 @@ export function Setup() { {mcpActiveVariant === 'http' && (
- Replace {'{'}{'{'}'BEARER_TOKEN{'}'}{'}'} with your daemon token + Replace {'{'}{'{'}'BEARER_TOKEN{'}'}{'}'} with your daemon token — the access password the daemon generates so only your own tools can call it ) : ( <> - - )} From 0f3d4855f242911118a23e72a9acb3e507919de3 Mon Sep 17 00:00:00 2001 From: "A.R." Date: Fri, 12 Jun 2026 11:08:30 +0300 Subject: [PATCH 13/19] fix(webview): add hostname field for Cloudflare Named Tunnel setup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Selecting cf-named and pressing Enable always dead-ended with 'Named tunnel requires a hostname (domain)' because the Setup tab rendered no hostname input for that provider — only ngrok had fields — and handleEnableTunnel sent the ngrok domain value for every provider. - Render a Hostname input when cf-named is selected (helper text: Cloudflare-managed domain, one-time browser login, leave empty to reuse an existing tunnel config). - Send the per-provider domain value from handleEnableTunnel. - ngrok's Reserved Domain field was also unreachable once an authtoken was stored (it lived inside the !ngrokAuthtokenSet block) — it now always renders for ngrok. - Host fallback: _ensureCfNamedSetup prompts via showInputBox when the hostname is still missing (flow reachable from commands that never showed the field), and the post-setup enable retry sends the RESOLVED hostname instead of the original empty originalMsg.domain. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 18 ++++ .../src/webview/DashboardProvider.ts | 21 ++++- packages/webview/src/tabs/Setup.tsx | 91 ++++++++++++++----- 3 files changed, 102 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e864da..595aed9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,24 @@ Check [Keep a Changelog](http://keepachangelog.com/) for recommendations on how ## [Unreleased] +### Setup tab — Cloudflare Named Tunnel hostname field (2026-06-12) + +Selecting **Cloudflare Named Tunnel** and pressing Enable always failed with +"Named tunnel requires a hostname (domain)" — the Setup tab never rendered a +field to enter one (only ngrok had inputs). Fixes: + +- New **Hostname** input shown when `cf-named` is selected, with helper text + (Cloudflare-managed domain, one-time browser login, empty = reuse existing + config) ([`Setup.tsx`](packages/webview/src/tabs/Setup.tsx)). +- `handleEnableTunnel` now sends the hostname for `cf-named` (it previously + sent the ngrok field's value for every provider). +- Same bug class for ngrok: the **Reserved Domain** field was hidden once an + authtoken was stored — it now always renders for ngrok. +- Host-side fallback: if the setup flow is reached without a hostname (e.g. + via a command), an input box prompts for it instead of dead-ending; the + post-setup enable retry uses the resolved hostname (previously re-sent the + original empty value) ([`DashboardProvider.ts`](packages/extension/src/webview/DashboardProvider.ts)). + ### Daemon Stop reliability + dashboard UI/UX hardening (2026-06-12) **Daemon Stop button** — root-caused "stop sometimes does nothing": diff --git a/packages/extension/src/webview/DashboardProvider.ts b/packages/extension/src/webview/DashboardProvider.ts index 010994d..bd8df0f 100644 --- a/packages/extension/src/webview/DashboardProvider.ts +++ b/packages/extension/src/webview/DashboardProvider.ts @@ -1129,8 +1129,20 @@ export class DashboardProvider implements vscode.WebviewViewProvider { } // Step 2: Create tunnel (no-op if already configured) - const hostname = originalMsg.domain; - if (!hostname) throw new Error('Named tunnel requires a hostname (domain). Enter it in the tunnel settings.'); + let hostname = originalMsg.domain; + if (!hostname) { + // Last-resort prompt — the Setup tab has a Hostname field, but the + // flow can also be reached from commands that never showed it. + hostname = await vscode.window.showInputBox({ + title: 'Cloudflare Named Tunnel', + prompt: 'Hostname for the tunnel — a domain you manage in Cloudflare', + placeHolder: 'mcp.your-domain.com', + ignoreFocusOut: true, + validateInput: (v) => (v.trim().length === 0 ? 'Hostname is required for first-time setup' : undefined), + }); + hostname = hostname?.trim() || undefined; + } + if (!hostname) throw new Error('Named tunnel requires a hostname (domain). Enter it in the Hostname field under the tunnel provider in the Setup tab.'); progress.report({ message: `Creating tunnel for ${hostname}…` }); const createResp = await fetch(`${base}/daemon/tunnel/named-create`, { method: 'POST', headers, @@ -1142,11 +1154,12 @@ export class DashboardProvider implements vscode.WebviewViewProvider { throw new Error(`Tunnel creation failed: ${b.error ?? createResp.status}`); } - // Step 3: Retry enable-tunnel now that setup is complete + // Step 3: Retry enable-tunnel now that setup is complete — with the + // RESOLVED hostname (originalMsg.domain may have been empty). progress.report({ message: 'Starting tunnel…' }); const enableResp = await fetch(`${base}/daemon/enable-tunnel`, { method: 'POST', headers, - body: JSON.stringify({ provider: originalMsg.provider, domain: originalMsg.domain }), + body: JSON.stringify({ provider: originalMsg.provider, domain: hostname }), signal: AbortSignal.timeout(90_000), }); if (!enableResp.ok) { diff --git a/packages/webview/src/tabs/Setup.tsx b/packages/webview/src/tabs/Setup.tsx index 6d27c65..5e78ff1 100644 --- a/packages/webview/src/tabs/Setup.tsx +++ b/packages/webview/src/tabs/Setup.tsx @@ -246,6 +246,7 @@ export function Setup() { const [selectedProvider, setSelectedProvider] = React.useState<'cf-quick' | 'ngrok' | 'cf-named'>('cf-quick'); const [ngrokAuthtokenInput, setNgrokAuthtokenInput] = React.useState(''); const [ngrokDomainInput, setNgrokDomainInput] = React.useState(''); + const [namedHostnameInput, setNamedHostnameInput] = React.useState(''); const [copiedUrl, setCopiedUrl] = React.useState(false); const [copiedDaemonUrl, setCopiedDaemonUrl] = React.useState(false); const [copiedToken, setCopiedToken] = React.useState(false); @@ -278,7 +279,11 @@ export function Setup() { const authtoken = (selectedProvider === 'ngrok' && ngrokAuthtokenInput) ? ngrokAuthtokenInput : undefined; - enableTunnel(selectedProvider, authtoken, ngrokDomainInput || undefined); + const domain = + selectedProvider === 'cf-named' ? (namedHostnameInput.trim() || undefined) + : selectedProvider === 'ngrok' ? (ngrokDomainInput.trim() || undefined) + : undefined; + enableTunnel(selectedProvider, authtoken, domain); }; const handleDisableTunnel = () => { @@ -504,19 +509,51 @@ export function Setup() {
- {/* ngrok authtoken input (shown when ngrok selected + no token stored) */} - {selectedProvider === 'ngrok' && !tunnel?.ngrokAuthtokenSet && ( + {/* ngrok inputs — authtoken only until one is stored; domain always */} + {selectedProvider === 'ngrok' && (
-
+ )} + + {/* Cloudflare named tunnel — hostname is required for first-time setup */} + {selectedProvider === 'cf-named' && ( +
+ setNgrokDomainInput(e.target.value)} + placeholder="mcp.your-domain.com" + aria-describedby="cf-named-hostname-helper" + value={namedHostnameInput} + onChange={e => setNamedHostnameInput(e.target.value)} style={{ width: '100%', boxSizing: 'border-box', @@ -555,8 +592,14 @@ export function Setup() { borderRadius: 'var(--radius-md)', padding: '4px 8px', color: 'var(--fg)', + marginBottom: 4, }} /> +
+ A domain you manage in Cloudflare (the tunnel gets a permanent URL there). + Required the first time — setup opens a one-time Cloudflare login in your browser. + Leave empty to reuse an already-configured tunnel. +
)} From d3e1dc9d3ba57d2f19ce1793a1961b03d7fb5d64 Mon Sep 17 00:00:00 2001 From: "A.R." Date: Fri, 12 Jun 2026 11:32:32 +0300 Subject: [PATCH 14/19] =?UTF-8?q?fix(webview):=20dead-end=20sweep=20?= =?UTF-8?q?=E2=80=94=20ngrok=20token=20update=20path,=20prompt=20editor=20?= =?UTF-8?q?feedback,=20form=20state,=20cosmetics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second-pass audit targeting the cf-named-hostname bug class (UI flows missing the affordances their handlers require) confirmed 12 findings: - ngrok authtoken was unreachable after first save: the input was gated on !ngrokAuthtokenSet and setNgrokAuthtoken existed in the store but was never wired to any UI. Stored state now shows an 'Auth token stored' chip with a Replace… flow (input + Save/Cancel) so the token can be rotated without disabling the tunnel. - PromptEditor Save/Reset/Delete gave no in-flight feedback and were clickable repeatedly; they now disable with aria-busy while any action is pending. - Copy Token now tracks pending state and disables; Rotate Token is daemon-scoped (beginDaemonAction) and disables with daemon controls; Copy PAT disables while busy. - Credentials form: email cleared on save alongside password/OTP, and Cancel resets all fields — no stale pre-filled email on reopen. - Cosmetics: input padding unified (6px 10px), helper text color standardized to --fg-subtle, daemon button gap 6->8, chip casing normalized (Ready/Missing/Via extension), uppercase label styles extracted to .uppercase-label, ngrok section spacing made state-independent. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 25 +++++++ packages/webview/src/components/IdeCard.tsx | 8 +-- packages/webview/src/store.ts | 5 +- packages/webview/src/styles.css | 8 +++ packages/webview/src/tabs/Prompts.tsx | 12 ++-- packages/webview/src/tabs/Settings.tsx | 11 ++- packages/webview/src/tabs/Setup.tsx | 74 ++++++++++++++++----- 7 files changed, 112 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 595aed9..b0d07e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,31 @@ Check [Keep a Changelog](http://keepachangelog.com/) for recommendations on how ## [Unreleased] +### Dashboard — second-pass dead-end & cosmetics sweep (2026-06-12) + +A targeted second audit (flow dead-ends, unreachable conditional UI, stale +affordances, cosmetics) confirmed 12 more issues; all fixed: + +- **ngrok auth token now has an update path** — once a token was stored, the + input vanished forever (`!ngrokAuthtokenSet` gate) and the existing + `setNgrokAuthtoken` store action was never wired to any UI. A "token + stored" chip with a **Replace…** flow now lets users rotate/update the + token without disabling the tunnel ([`Setup.tsx`](packages/webview/src/tabs/Setup.tsx)). +- **PromptEditor feedback** — Save/Reset/Delete now disable (+`aria-busy`) + while the action is in flight ([`Prompts.tsx`](packages/webview/src/tabs/Prompts.tsx)). +- **Token/PAT buttons** — Copy Token tracks pending state and disables; + Rotate is daemon-scoped (`beginDaemonAction`) and disables with the other + daemon controls; Copy PAT disables while busy ([`store.ts`](packages/webview/src/store.ts)). +- **Credentials form state** — email is now cleared along with password/OTP + on save, and Cancel resets all fields (stale pre-filled email no longer + reappears on "Update credentials") ([`Settings.tsx`](packages/webview/src/tabs/Settings.tsx)). +- **Cosmetics:** input padding unified at `6px 10px` across tunnel/PAT/ + credentials fields; helper text consistently `--fg-subtle`; daemon button + row gap aligned to 8px; chip casing normalized (Ready/Missing/Via + extension); repeated uppercase-label inline styles extracted to a shared + `.uppercase-label` class; the ngrok section's state-dependent spacing + asymmetry (introduced by the hostname-field fix) removed. + ### Setup tab — Cloudflare Named Tunnel hostname field (2026-06-12) Selecting **Cloudflare Named Tunnel** and pressing Enable always failed with diff --git a/packages/webview/src/components/IdeCard.tsx b/packages/webview/src/components/IdeCard.tsx index 6a1dc9b..cc25339 100644 --- a/packages/webview/src/components/IdeCard.tsx +++ b/packages/webview/src/components/IdeCard.tsx @@ -101,7 +101,7 @@ export function IdeCard({ status, onSetup, onUnconfigure, loading }: IdeCardProp MCP {status.mcpConfigured ? 'configured' : 'not configured'} - {status.mcpConfigured ? 'ready' : 'missing'} + {status.mcpConfigured ? 'Ready' : 'Missing'}
)} @@ -113,7 +113,7 @@ export function IdeCard({ status, onSetup, onUnconfigure, loading }: IdeCardProp Formula · Script · Automation - via extension + Via extension
)} @@ -125,7 +125,7 @@ export function IdeCard({ status, onSetup, onUnconfigure, loading }: IdeCardProp LSP {status.lspConfigured ? 'configured' : 'not configured'} - {status.lspConfigured ? 'ready' : 'missing'} + {status.lspConfigured ? 'Ready' : 'Missing'}
)} @@ -138,7 +138,7 @@ export function IdeCard({ status, onSetup, onUnconfigure, loading }: IdeCardProp LSP {status.lspConfigured ? 'configured' : 'not configured'} - {status.lspConfigured ? 'ready' : 'missing'} + {status.lspConfigured ? 'Ready' : 'Missing'}
)} diff --git a/packages/webview/src/store.ts b/packages/webview/src/store.ts index f66580b..dc0128b 100644 --- a/packages/webview/src/store.ts +++ b/packages/webview/src/store.ts @@ -302,12 +302,15 @@ export const useStore = create((set, get) => { copyBearerToken: () => { const id = randomId(); + beginAction(id); sendToExtension({ type: 'daemon:copy-bearer-token', id }); }, rotateToken: () => { + // Daemon-scoped: rotating invalidates connected clients, so daemon + // controls should reflect the in-flight rotation too. const id = randomId(); - beginAction(id); + beginDaemonAction(id); sendToExtension({ type: 'daemon:rotate-token', id }); }, diff --git a/packages/webview/src/styles.css b/packages/webview/src/styles.css index 5b15867..73d1dff 100644 --- a/packages/webview/src/styles.css +++ b/packages/webview/src/styles.css @@ -93,6 +93,14 @@ button[aria-disabled="true"] { /* ─── Design System Classes ─── */ +/* Small uppercase form/section labels — one source of truth for the + fontSize/tracking so design changes don't require editing every tab. */ +.uppercase-label { + font-size: 0.7rem; + text-transform: uppercase; + letter-spacing: 0.1em; +} + /* Skeleton loading rows (Setup tab while IDE detection runs) */ .skeleton-row { height: 38px; diff --git a/packages/webview/src/tabs/Prompts.tsx b/packages/webview/src/tabs/Prompts.tsx index 7b900e4..5139ddc 100644 --- a/packages/webview/src/tabs/Prompts.tsx +++ b/packages/webview/src/tabs/Prompts.tsx @@ -85,7 +85,8 @@ function PromptEditor({ isNew: boolean; onBack: () => void; }) { - const { savePrompt, deletePrompt, resetPrompt } = useStore(); + const { savePrompt, deletePrompt, resetPrompt, pendingActions } = useStore(); + const busy = pendingActions.size > 0; const [name, setName] = useState(initial.name); const [desc, setDesc] = useState(initial.description); const [args, setArgs] = useState(initial.arguments); @@ -221,21 +222,22 @@ function PromptEditor({
{initial.isBuiltin && initial.isModified && ( - )} {!initial.isBuiltin && ( - )} diff --git a/packages/webview/src/tabs/Settings.tsx b/packages/webview/src/tabs/Settings.tsx index bf753f4..d1c9fdc 100644 --- a/packages/webview/src/tabs/Settings.tsx +++ b/packages/webview/src/tabs/Settings.tsx @@ -90,11 +90,16 @@ export function Settings() { sendToExtension({ type: 'setting:change', key: 'auth.refreshIntervalHours', value: Number(e.target.value) }); }; + const resetCredsForm = () => { + setEmail(''); + setPassword(''); + setOtpSecret(''); + }; + const handleSaveCredentials = () => { if (!email || !password) return; saveCredentials(email, password, otpSecret); - setPassword(''); - setOtpSecret(''); + resetCredsForm(); setShowCreds(false); }; @@ -341,7 +346,7 @@ export function Settings() { + +
+ )} +
ngrok tunnels must be manually re-enabled after daemon restarts (authtoken stored in VS Code SecretStorage is not accessible to the daemon at startup).
+ ) : ( +
+ Auth token stored +
+ +
)} -