Skip to content

Commit 680fa65

Browse files
authored
feat(cli): gate CLI installation behind feature flag (#1544)
* feat(cli): gate PATH configuration behind feature flag Skip writing to shell RC files when cli_validation_enforce flag is disabled, while still detecting and logging the user's shell to Sentry for analytics purposes. * feat(cli): gate PATH configuration behind feature flag Wrap configureShellPath in a dedicated Sentry span to track shell detection analytics. Skip writing to shell RC files (Unix) and Windows registry when cli_validation_enforce flag is disabled. * feat(cli): gate CLI installation behind feature flag Skip symlink creation, marker file, and PATH configuration when cli_validation_enforce flag is disabled. Add Sentry span for configureShellPath to track shell detection analytics.
1 parent 1a13add commit 680fa65

File tree

4 files changed

+230
-25
lines changed

4 files changed

+230
-25
lines changed

main/src/cli/__tests__/path-configurator.test.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,40 @@ vi.mock('../../logger', () => ({
7373
},
7474
}))
7575

76+
const { mockSpan } = vi.hoisted(() => ({
77+
mockSpan: {
78+
setAttribute: vi.fn(),
79+
setAttributes: vi.fn(),
80+
end: vi.fn(),
81+
},
82+
}))
83+
84+
vi.mock('@sentry/electron/main', () => ({
85+
startSpanManual: vi.fn((_options, callback) => callback(mockSpan)),
86+
}))
87+
88+
const { mockGetFeatureFlag } = vi.hoisted(() => ({
89+
mockGetFeatureFlag: vi.fn().mockReturnValue(true),
90+
}))
91+
92+
vi.mock('../../feature-flags', () => ({
93+
getFeatureFlag: mockGetFeatureFlag,
94+
}))
95+
96+
vi.mock('../../../../utils/feature-flags', () => ({
97+
featureFlagKeys: {
98+
CLI_VALIDATION_ENFORCE: 'cli_validation_enforce',
99+
},
100+
}))
101+
76102
describe('path-configurator', () => {
77103
beforeEach(() => {
78104
vol.reset()
79105
vi.clearAllMocks()
106+
// Reset span mock
107+
mockSpan.setAttribute.mockClear()
108+
mockSpan.setAttributes.mockClear()
109+
mockSpan.end.mockClear()
80110
// Default to non-Windows
81111
vi.stubGlobal('process', {
82112
...process,
@@ -85,6 +115,8 @@ describe('path-configurator', () => {
85115
})
86116
// Default mock for execAsync (shell detection)
87117
mockExecAsync.mockResolvedValue({ stdout: '', stderr: '' })
118+
// Default feature flag to enabled
119+
mockGetFeatureFlag.mockReturnValue(true)
88120
})
89121

90122
afterEach(() => {
@@ -186,6 +218,36 @@ export PATH="$HOME/.toolhive/bin:$PATH"
186218
expect(result.success).toBe(true)
187219
expect(vol.existsSync('/home/testuser/.config/fish')).toBe(true)
188220
})
221+
222+
it('skips PATH modification when feature flag is disabled but still records shell in span', async () => {
223+
mockGetFeatureFlag.mockReturnValue(false)
224+
225+
vol.fromJSON({
226+
'/home/testuser/.zshrc': '# My zsh config\nalias ll="ls -la"\n',
227+
})
228+
229+
const result = await configureShellPath()
230+
231+
expect(result.success).toBe(false)
232+
expect(result.modifiedFiles).toHaveLength(0)
233+
// Should still record detected shell in span
234+
expect(mockSpan.setAttribute).toHaveBeenCalledWith(
235+
'cli.detected_shell',
236+
'zsh'
237+
)
238+
expect(mockSpan.setAttribute).toHaveBeenCalledWith(
239+
'cli.feature_flag_enabled',
240+
false
241+
)
242+
expect(mockSpan.setAttributes).toHaveBeenCalledWith({
243+
'cli.path_configured': false,
244+
'cli.skipped_reason': 'feature_flag_disabled',
245+
})
246+
expect(mockSpan.end).toHaveBeenCalled()
247+
// File should not be modified
248+
const content = vol.readFileSync('/home/testuser/.zshrc', 'utf8')
249+
expect(content).not.toContain('# Added by ToolHive Studio')
250+
})
189251
})
190252

191253
describe('removeShellPath', () => {
@@ -388,5 +450,28 @@ export PATH="$HOME/.toolhive/bin:$PATH"
388450

389451
expect(result.isConfigured).toBe(false)
390452
})
453+
454+
it('skips PATH modification when feature flag is disabled on Windows', async () => {
455+
mockGetFeatureFlag.mockReturnValue(false)
456+
mockExecAsync.mockResolvedValue({ stdout: '', stderr: '' })
457+
458+
const result = await configureShellPath()
459+
460+
expect(result.success).toBe(false)
461+
expect(result.modifiedFiles).toHaveLength(0)
462+
// Should record in span
463+
expect(mockSpan.setAttribute).toHaveBeenCalledWith(
464+
'cli.feature_flag_enabled',
465+
false
466+
)
467+
expect(mockSpan.setAttribute).toHaveBeenCalledWith('cli.is_windows', true)
468+
expect(mockSpan.setAttributes).toHaveBeenCalledWith({
469+
'cli.path_configured': false,
470+
'cli.skipped_reason': 'feature_flag_disabled',
471+
})
472+
expect(mockSpan.end).toHaveBeenCalled()
473+
// PowerShell should NOT be called
474+
expect(mockExecAsync).not.toHaveBeenCalled()
475+
})
391476
})
392477
})

main/src/cli/__tests__/validation.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,20 @@ vi.mock('@sentry/electron/main', () => ({
5656
startSpan: vi.fn((_options, callback) => callback()),
5757
}))
5858

59+
const { mockGetFeatureFlag } = vi.hoisted(() => ({
60+
mockGetFeatureFlag: vi.fn().mockReturnValue(true),
61+
}))
62+
63+
vi.mock('../../feature-flags', () => ({
64+
getFeatureFlag: mockGetFeatureFlag,
65+
}))
66+
67+
vi.mock('../../../../utils/feature-flags', () => ({
68+
featureFlagKeys: {
69+
CLI_VALIDATION_ENFORCE: 'cli_validation_enforce',
70+
},
71+
}))
72+
5973
import { detectExternalCli, getCliInfo } from '../cli-detection'
6074
import { readMarkerFile, createMarkerForDesktopInstall } from '../marker-file'
6175
import {
@@ -93,6 +107,8 @@ describe('validation', () => {
93107
modifiedFiles: [],
94108
pathEntry: '/home/testuser/.toolhive/bin',
95109
})
110+
// Default feature flag to enabled
111+
mockGetFeatureFlag.mockReturnValue(true)
96112
})
97113

98114
afterEach(() => {
@@ -335,6 +351,33 @@ describe('validation', () => {
335351

336352
expect(result.status).toBe('fresh-install')
337353
})
354+
355+
it('skips symlink and marker creation when feature flag is disabled', async () => {
356+
mockGetFeatureFlag.mockReturnValue(false)
357+
358+
const result = await handleValidationResult(
359+
{ status: 'fresh-install' },
360+
'darwin'
361+
)
362+
363+
expect(mockCreateSymlink).not.toHaveBeenCalled()
364+
expect(mockCreateMarkerForDesktopInstall).not.toHaveBeenCalled()
365+
expect(mockConfigureShellPath).not.toHaveBeenCalled()
366+
expect(result.status).toBe('valid')
367+
})
368+
369+
it('skips symlink creation for symlink-missing when feature flag is disabled', async () => {
370+
mockGetFeatureFlag.mockReturnValue(false)
371+
372+
const result = await handleValidationResult(
373+
{ status: 'symlink-missing' },
374+
'darwin'
375+
)
376+
377+
expect(mockCreateSymlink).not.toHaveBeenCalled()
378+
expect(mockCreateMarkerForDesktopInstall).not.toHaveBeenCalled()
379+
expect(result.status).toBe('valid')
380+
})
338381
})
339382

340383
describe('repairCliSymlink', () => {

main/src/cli/path-configurator.ts

Lines changed: 81 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import path from 'node:path'
1414
import { homedir } from 'node:os'
1515
import { exec } from 'node:child_process'
1616
import { promisify } from 'node:util'
17+
import * as Sentry from '@sentry/electron/main'
1718
import {
1819
getShellRcFiles,
1920
SHELL_PATH_ENTRY,
@@ -22,6 +23,8 @@ import {
2223
} from './constants'
2324
import type { PathConfigStatus } from './types'
2425
import log from '../logger'
26+
import { getFeatureFlag } from '../feature-flags'
27+
import { featureFlagKeys } from '../../../utils/feature-flags'
2528

2629
const execAsync = promisify(exec)
2730

@@ -173,42 +176,95 @@ export async function configureShellPath(): Promise<{
173176
success: boolean
174177
modifiedFiles: string[]
175178
}> {
176-
if (process.platform === 'win32') {
177-
return configureWindowsPath()
178-
}
179+
return Sentry.startSpanManual(
180+
{
181+
name: 'CLI configure shell PATH',
182+
op: 'cli.configure_path',
183+
attributes: {
184+
'analytics.source': 'tracking',
185+
'analytics.type': 'event',
186+
'cli.platform': process.platform,
187+
},
188+
},
189+
async (span) => {
190+
// Check if PATH configuration is enabled via feature flag
191+
const isPathConfigEnabled = getFeatureFlag(
192+
featureFlagKeys.CLI_VALIDATION_ENFORCE
193+
)
179194

180-
const shellRcFiles = getShellRcFiles()
181-
const modifiedFiles: string[] = []
195+
span.setAttribute('cli.feature_flag_enabled', isPathConfigEnabled)
182196

183-
const defaultShell = await detectDefaultShell()
184-
log.info(`Detected default shell: ${defaultShell}`)
197+
if (process.platform === 'win32') {
198+
span.setAttribute('cli.is_windows', true)
185199

186-
const defaultShellFiles = shellRcFiles[defaultShell] ?? []
187-
const isFish = defaultShell === 'fish'
200+
if (!isPathConfigEnabled) {
201+
log.info('PATH configuration disabled by feature flag, skipping')
202+
span.setAttributes({
203+
'cli.path_configured': false,
204+
'cli.skipped_reason': 'feature_flag_disabled',
205+
})
206+
span.end()
207+
return { success: false, modifiedFiles: [] }
208+
}
188209

189-
for (const rcFile of defaultShellFiles) {
190-
if (addPathToFile(rcFile, isFish)) {
191-
modifiedFiles.push(rcFile)
192-
}
193-
}
210+
const result = await configureWindowsPath()
211+
span.setAttribute('cli.path_configured', result.success)
212+
span.end()
213+
return result
214+
}
194215

195-
const otherShells = Object.entries(shellRcFiles).filter(
196-
([shell]) => shell !== defaultShell
197-
)
216+
const shellRcFiles = getShellRcFiles()
217+
const modifiedFiles: string[] = []
218+
219+
const defaultShell = await detectDefaultShell()
220+
log.info(`Detected default shell: ${defaultShell}`)
221+
222+
// Record detected shell in span for analytics
223+
span.setAttribute('cli.detected_shell', defaultShell)
198224

199-
for (const [shell, files] of otherShells) {
200-
const shellIsFish = shell === 'fish'
225+
if (!isPathConfigEnabled) {
226+
log.info('PATH configuration disabled by feature flag, skipping')
227+
span.setAttributes({
228+
'cli.path_configured': false,
229+
'cli.skipped_reason': 'feature_flag_disabled',
230+
})
231+
span.end()
232+
return { success: false, modifiedFiles: [] }
233+
}
234+
235+
const defaultShellFiles = shellRcFiles[defaultShell] ?? []
236+
const isFish = defaultShell === 'fish'
201237

202-
for (const rcFile of files) {
203-
if (existsSync(rcFile) && addPathToFile(rcFile, shellIsFish)) {
204-
if (!modifiedFiles.includes(rcFile)) {
238+
for (const rcFile of defaultShellFiles) {
239+
if (addPathToFile(rcFile, isFish)) {
205240
modifiedFiles.push(rcFile)
206241
}
207242
}
208-
}
209-
}
210243

211-
return { success: true, modifiedFiles }
244+
const otherShells = Object.entries(shellRcFiles).filter(
245+
([shell]) => shell !== defaultShell
246+
)
247+
248+
for (const [shell, files] of otherShells) {
249+
const shellIsFish = shell === 'fish'
250+
251+
for (const rcFile of files) {
252+
if (existsSync(rcFile) && addPathToFile(rcFile, shellIsFish)) {
253+
if (!modifiedFiles.includes(rcFile)) {
254+
modifiedFiles.push(rcFile)
255+
}
256+
}
257+
}
258+
}
259+
260+
span.setAttributes({
261+
'cli.path_configured': true,
262+
'cli.modified_files_count': modifiedFiles.length,
263+
})
264+
span.end()
265+
return { success: true, modifiedFiles }
266+
}
267+
)
212268
}
213269

214270
async function removeWindowsPath(): Promise<{

main/src/cli/validation.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import { getDesktopCliPath } from './constants'
1818
import type { ValidationResult } from '@common/types/cli'
1919
import type { CliAlignmentStatus, Platform } from './types'
2020
import log from '../logger'
21+
import { getFeatureFlag } from '../feature-flags'
22+
import { featureFlagKeys } from '../../../utils/feature-flags'
2123

2224
export async function validateCliAlignment(
2325
platform: Platform = process.platform as Platform
@@ -206,6 +208,25 @@ export async function handleValidationResult(
206208
// These cases can be auto-fixed without user interaction
207209
case 'symlink-missing':
208210
case 'fresh-install': {
211+
// Check if CLI installation is enabled via feature flag
212+
const isCliInstallEnabled = getFeatureFlag(
213+
featureFlagKeys.CLI_VALIDATION_ENFORCE
214+
)
215+
216+
span.setAttribute('cli.feature_flag_enabled', isCliInstallEnabled)
217+
218+
if (!isCliInstallEnabled) {
219+
log.info(
220+
'CLI installation disabled by feature flag, skipping symlink and marker creation'
221+
)
222+
span.setAttributes({
223+
'cli.output_status': 'skipped',
224+
'cli.skipped_reason': 'feature_flag_disabled',
225+
})
226+
span.end()
227+
return { status: 'valid' }
228+
}
229+
209230
log.info('Performing fresh CLI installation...')
210231

211232
const symlinkResult = createSymlink(platform)

0 commit comments

Comments
 (0)