Skip to content

Commit c22df05

Browse files
committed
fix(cli): tighten update check eligibility
1 parent 4c24d3a commit c22df05

6 files changed

Lines changed: 73 additions & 19 deletions

File tree

apps/docs/content/docs/cli/configuration.mdx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,16 +147,18 @@ containing username/password userinfo, such as
147147
`https://user:password@registry.example`, is rejected and no update check is
148148
made.
149149

150-
Malformed and non-HTTP(S) configured registry values also disable the update
151-
check rather than making an unexpected request to the public registry.
150+
An empty or whitespace-only `npm_config_registry` is treated as unset, so the
151+
public registry remains the default. Non-empty malformed and non-HTTP(S) values
152+
disable the update check rather than making an unexpected public request.
152153

153154
The notice is skipped entirely when:
154155

155156
- `SIM_NO_UPDATE_CHECK` is set to anything but `0` or `false`
156157
- stderr is not a terminal, so redirected and piped output is never affected
157158
- a CI environment variable is present (`CI`, `GITHUB_ACTIONS`, `JENKINS_URL`,
158159
`TEAMCITY_VERSION`, `BUILDKITE`)
159-
- the CLI is running under `npx`, which resolves the newest version every time
160+
- the CLI is running under `npm exec` or `npx`, which may use a project-local or
161+
ephemeral package where global-install advice is inappropriate
160162
- the CLI is running from a checkout of the sim repository, whose version
161163
deliberately trails the published one
162164
- the installed version is a prerelease

apps/docs/content/docs/cli/troubleshooting.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ one installs a second copy instead of replacing the executable on your `PATH`:
120120

121121
The CLI can also tell you this through a cached daily check on eligible
122122
invocations, and the command it prints already matches your installation. It
123-
stays quiet when stderr is redirected, in CI, and under `npx`.
123+
stays quiet when stderr is redirected, in CI, and under `npm exec` or `npx`.
124124

125125
## An update notice appears in output I am parsing
126126

packages/sim-cli/README.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -272,9 +272,10 @@ Apart from the configured registry URL, it sends only its own version and never
272272
your Sim API key. If `npm_config_registry` points at a private mirror, its query
273273
string is preserved, including any query-string credentials. Registry URLs
274274
containing username/password userinfo are rejected. Set
275-
`SIM_NO_UPDATE_CHECK=1` to turn it off; malformed or non-HTTP(S) configured
276-
registry values also fail closed. The full list of cases where it stays quiet
277-
is in the [configuration guide](https://docs.sim.ai/cli/configuration).
275+
`SIM_NO_UPDATE_CHECK=1` to turn it off. Empty or whitespace-only registry values
276+
use the public default; non-empty malformed or non-HTTP(S) values fail closed.
277+
The full list of cases where it stays quiet is in the
278+
[configuration guide](https://docs.sim.ai/cli/configuration).
278279

279280
## Documentation
280281

packages/sim-cli/src/update/check.process.test.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ function runChild(
8787
JENKINS_URL: '0',
8888
NODE_USE_ENV_PROXY: '0',
8989
NO_PROXY: '127.0.0.1,localhost',
90+
npm_command: '',
9091
TEAMCITY_VERSION: '0',
9192
...options.env,
9293
SIM_CONFIG_DIR: configDir,
@@ -364,11 +365,12 @@ it.skipIf(!SUPPORTS_PROXY_FLAG)(
364365
10_000
365366
)
366367

367-
it('keeps a configured registry credential out of probe argv and environment', async () => {
368+
it('keeps CLI credentials out of probe argv and environment', async () => {
368369
const inspectionPath = join(temporaryDir, 'probe-inspection.json')
369370
const preloadDir = join(temporaryDir, 'probe preload')
370371
const preloadPath = join(preloadDir, 'inspect-probe.cjs')
371-
const sentinel = 'registry-secret-sentinel'
372+
const registrySentinel = 'registry-secret-sentinel'
373+
const apiKeySentinel = 'api-key-secret-sentinel'
372374
let requestPath: string | undefined
373375
mkdirSync(preloadDir)
374376
writeFileSync(
@@ -395,24 +397,27 @@ it('keeps a configured registry credential out of probe argv and environment', a
395397
response.end(JSON.stringify({ latest: '2.1.5' }))
396398
},
397399
async (origin) => {
398-
const registry = `${origin}?token=${sentinel}`
400+
const registry = `${origin}?token=${registrySentinel}`
399401
const result = await runChild(entrypoint, registry, join(temporaryDir, 'config-credential'), {
400402
env: {
401403
NODE_OPTIONS: `--require="${preloadPath}"`,
402404
NPM_CONFIG_REGISTRY: registry,
403405
PROBE_INSPECTION_PATH: inspectionPath,
406+
SIM_API_KEY: apiKeySentinel,
404407
},
405408
useProcessEnv: true,
406409
})
407410

408411
expect(result).toMatchObject({ code: 0, signal: null, stderr: '' })
409-
expect(requestPath).toBe(`/-/package/sim/dist-tags?token=${sentinel}`)
412+
expect(requestPath).toBe(`/-/package/sim/dist-tags?token=${registrySentinel}`)
410413
const inspection = JSON.parse(readFileSync(inspectionPath, 'utf8')) as {
411414
argv: string[]
412415
environmentValues: string[]
413416
execArgv: string[]
414417
}
415-
expect(JSON.stringify(inspection)).not.toContain(sentinel)
418+
const serializedInspection = JSON.stringify(inspection)
419+
expect(serializedInspection).not.toContain(registrySentinel)
420+
expect(serializedInspection).not.toContain(apiKeySentinel)
416421
}
417422
)
418423
}, 10_000)

packages/sim-cli/src/update/check.test.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,34 @@ describe('when the notice is suppressed', () => {
177177
it.each([
178178
'/Users/x/.npm/_npx/a1b2/node_modules/sim/dist/index.js',
179179
'C:\\Users\\x\\AppData\\Local\\npm-cache\\_npx\\a1b2\\node_modules\\sim\\dist\\index.js',
180-
])('says nothing under npx, which resolves the tag on every run (%s)', async (modulePath) => {
180+
])('says nothing for an npx cache installation (%s)', async (modulePath) => {
181181
await run({ modulePath })
182+
expect(fetched).toEqual([])
183+
expect(notices).toEqual([])
184+
})
185+
186+
it.each([
187+
'/Users/x/project/node_modules/sim/dist/index.js',
188+
'C:\\Users\\x\\project\\node_modules\\sim\\dist\\index.js',
189+
])('says nothing when npm exec resolves a project-local dependency (%s)', async (modulePath) => {
190+
await run({ env: { npm_command: 'exec' }, modulePath })
191+
expect(fetched).toEqual([])
192+
expect(notices).toEqual([])
193+
})
194+
195+
it.each([
196+
{
197+
cwd: '/Users/x/project/packages/app',
198+
modulePath: '/Users/x/project/node_modules/sim/dist/index.js',
199+
},
200+
{
201+
cwd: 'C:\\Users\\x\\project\\packages\\app',
202+
modulePath:
203+
'C:\\Users\\x\\project\\node_modules\\.pnpm\\sim@2.1.2\\node_modules\\sim\\dist\\index.js',
204+
},
205+
])('says nothing from a project-local install at $modulePath', async ({ cwd, modulePath }) => {
206+
await run({ cwd, modulePath })
207+
expect(fetched).toEqual([])
182208
expect(notices).toEqual([])
183209
})
184210

packages/sim-cli/src/update/check.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ interface UpdateCacheEntry {
8787
const CACHE_VERSION = 1
8888

8989
export interface UpdateCheckOptions {
90+
/** Current working directory. Injected so project-local installation detection is testable. */
91+
cwd?: string
9092
currentVersion?: string
9193
env?: NodeJS.ProcessEnv
9294
/** Whether stderr is a terminal. Injected so the suppression rule is testable. */
@@ -117,10 +119,26 @@ function isEnabled(value: string | undefined): boolean {
117119
return normalized !== '' && normalized !== '0' && normalized !== 'false'
118120
}
119121

120-
/** Skips npx, which resolves latest, and checkouts, whose manifest trails npm. */
121-
function isUnadvisableInstall(modulePath: string): boolean {
122+
/** Whether the package is installed in a node_modules tree above the working directory. */
123+
function isProjectLocalInstall(modulePath: string, cwd: string): boolean {
124+
const normalizedModulePath = normalizeModulePath(modulePath)
125+
const nodeModulesIndex = normalizedModulePath.indexOf('/node_modules/')
126+
if (nodeModulesIndex < 0) return false
127+
128+
const installRoot = normalizedModulePath.slice(0, nodeModulesIndex)
129+
const workingDirectory = normalizeModulePath(cwd).replace(/\/+$/, '')
130+
return workingDirectory === installRoot || workingDirectory.startsWith(`${installRoot}/`)
131+
}
132+
133+
/** Skips ephemeral, project-local, and checkout installs that global advice cannot update. */
134+
function isUnadvisableInstall(modulePath: string, env: NodeJS.ProcessEnv, cwd: string): boolean {
122135
const normalized = normalizeModulePath(modulePath)
123-
return normalized.includes('/_npx/') || normalized.includes('/packages/sim-cli/')
136+
return (
137+
env.npm_command === 'exec' ||
138+
normalized.includes('/_npx/') ||
139+
normalized.includes('/packages/sim-cli/') ||
140+
isProjectLocalInstall(modulePath, cwd)
141+
)
124142
}
125143

126144
/** Normalizes separators and case before installation-path comparisons. */
@@ -185,11 +203,12 @@ try {
185203
}
186204
`
187205

188-
/** Preserves proxy/TLS settings without copying the registry credential into the probe. */
206+
/** Preserves proxy/TLS settings without copying CLI credentials into the probe. */
189207
function registryProcessEnv(): NodeJS.ProcessEnv {
190208
const env = { ...process.env }
191209
for (const key of Object.keys(env)) {
192-
if (key.toLowerCase() === 'npm_config_registry') delete env[key]
210+
const normalized = key.toLowerCase()
211+
if (normalized === 'npm_config_registry' || normalized === 'sim_api_key') delete env[key]
193212
}
194213
return env
195214
}
@@ -421,12 +440,13 @@ export async function announceUpdateIfAvailable(options: UpdateCheckOptions = {}
421440
const env = options.env ?? process.env
422441
const isTty = options.isTty ?? process.stderr.isTTY === true
423442
const modulePath = options.modulePath ?? fileURLToPath(import.meta.url)
443+
const cwd = options.cwd ?? process.cwd()
424444
const now = options.now ?? new Date()
425445

426446
if (isEnabled(env.SIM_NO_UPDATE_CHECK)) return
427447
if (!isTty) return
428448
if (CI_VARIABLES.some((variable) => isEnabled(env[variable]))) return
429-
if (isUnadvisableInstall(modulePath)) return
449+
if (isUnadvisableInstall(modulePath, env, cwd)) return
430450

431451
const currentVersion = options.currentVersion ?? CLI_VERSION
432452
const current = parseStableVersion(currentVersion)

0 commit comments

Comments
 (0)