From 255292458881e4dcba3f6b990162390a4d8d939a Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Tue, 11 Aug 2026 17:33:07 +0530 Subject: [PATCH 01/14] feat(SDK-7250): capture the wdio config file in auto-captured logs The archive uploaded at onComplete carried only our own two debug logs, so triaging an App-A11y no-scan report meant asking the customer how they had configured the service. It now also carries a credential-redacted copy of their wdio config, the local config files it imports, and package.json. WebdriverIO keeps the config path in ConfigParser's private #configFilePath (v8 and v9 alike) and no service can reach it, so configCapture.ts resolves it through a ladder of fallbacks: the `config-path` key yargs leaves behind from `run `, the raw argv positional, rootDir, cwd, and finally a single unambiguous *.conf.* in either directory. Resolved once in onPrepare and published on the environment so the upload path never re-derives it from cwd -- that re-derivation is the bug SDK-5993 fixed in the Node SDK. Opt out with `disableAutoCaptureLogs: true` or BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS=true. The flag is mirrored onto the environment because the detached cleanup rescue calls uploadLogs with no options -- and since opting out leaves logsUploaded false, that rescue is armed on exactly the runs that opted out. Also fixes two latent archive bugs this made reachable: the staging directory is now per-run (the fixed tmpdir()/logs.tar names let concurrent runs clobber and unlink each other's archives) and archive entry names are de-duplicated (two captured files sharing a basename silently overwrote each other). Co-Authored-By: Claude Opus 5 (1M context) --- .../browserstack-service/src/configCapture.ts | 502 ++++++++++++++++++ .../browserstack-service/src/constants.ts | 40 ++ .../browserstack-service/src/exitHandler.ts | 5 +- packages/browserstack-service/src/launcher.ts | 17 +- packages/browserstack-service/src/types.ts | 9 + packages/browserstack-service/src/util.ts | 102 +++- .../tests/configCapture.test.ts | 354 ++++++++++++ .../tests/uploadLogsArchive.test.ts | 158 ++++++ 8 files changed, 1170 insertions(+), 17 deletions(-) create mode 100644 packages/browserstack-service/src/configCapture.ts create mode 100644 packages/browserstack-service/tests/configCapture.test.ts create mode 100644 packages/browserstack-service/tests/uploadLogsArchive.test.ts diff --git a/packages/browserstack-service/src/configCapture.ts b/packages/browserstack-service/src/configCapture.ts new file mode 100644 index 0000000..87d9be7 --- /dev/null +++ b/packages/browserstack-service/src/configCapture.ts @@ -0,0 +1,502 @@ +import fs from 'node:fs' +import path from 'node:path' + +import type { Options } from '@wdio/types' + +import { BStackLogger } from './bstackLogger.js' +import { + BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS, + BROWSERSTACK_WDIO_CONFIG_FILE_PATH, + BROWSERSTACK_WDIO_CONFIG_STRATEGY, + CAPTURE_CONFIG_IMPORT_DEPTH, + DEFAULT_WDIO_CONFIG_BASENAME, + MAX_CAPTURED_CONFIG_FILES, + MAX_CAPTURED_CONFIG_FILE_BYTES, + MAX_PACKAGE_JSON_WALK_UP, + REDACTED_KEYS, + SUPPORTED_WDIO_CONFIG_EXTENSIONS, + WDIO_CLI_SUBCOMMANDS +} from './constants.js' + +export interface CapturedFile { + /* archive entry name (basename, de-duplicated) */ + name: string + /* absolute path the content came from */ + sourcePath: string + content: string +} + +export interface ConfigPathResolution { + configPath?: string + /* which ladder rung answered — recorded on the SDK_UPLOAD_LOGS event */ + strategy?: string + /* why nothing was found, when configPath is undefined */ + reason?: string +} + +const isReadableFile = (filePath: string): boolean => { + try { + return fs.existsSync(filePath) && fs.statSync(filePath).isFile() + } catch { + return false + } +} + +/** + * WDIO hands us the config path exactly as the user typed it (relative or absolute), + * so every candidate is resolved against cwd — the same base the CLI itself uses + * (`create-wdio` formatConfigFilePaths). + */ +const resolveCandidate = (value: unknown): string | undefined => { + if (typeof value !== 'string' || value.trim() === '') { + return undefined + } + try { + const resolved = path.resolve(process.cwd(), value.trim()) + return isReadableFile(resolved) ? resolved : undefined + } catch { + return undefined + } +} + +const probeConfigBasename = (dir: string, basename: string): string | undefined => { + for (const ext of SUPPORTED_WDIO_CONFIG_EXTENSIONS) { + const candidate = path.join(dir, `${basename}${ext}`) + if (isReadableFile(candidate)) { + return candidate + } + } + return undefined +} + +/** + * Last-resort discovery: a directory containing exactly ONE `*.conf.` file is + * unambiguous. Two or more (e.g. `wdio.conf.ts` + `wdio.app.conf.ts`) is not, and we + * deliberately capture nothing rather than upload the wrong file. + */ +const scanForSingleConfig = (dir: string): { match?: string, ambiguous?: boolean } => { + try { + const matches = fs.readdirSync(dir) + .filter((entry) => SUPPORTED_WDIO_CONFIG_EXTENSIONS.includes(path.extname(entry))) + .filter((entry) => /\.conf(ig)?\.[^.]+$/i.test(entry)) + .map((entry) => path.join(dir, entry)) + .filter(isReadableFile) + + if (matches.length === 1) { + return { match: matches[0] } + } + return { ambiguous: matches.length > 1 } + } catch { + return {} + } +} + +/** + * Scan the raw CLI args for the config positional. + * + * Covers `wdio ` (bare form), where WDIO strips the path before the config + * object is built. A token is skipped when the PREVIOUS token is a flag, otherwise + * `wdio run conf.js --spec ./tests/a.js` would resolve to the spec file. + */ +const scanArgvForConfig = (argv: string[]): string | undefined => { + for (let i = 0; i < argv.length; i++) { + const arg = argv[i] + const previous = i > 0 ? argv[i - 1] : undefined + + if (!arg || arg.startsWith('-')) { + continue + } + if (WDIO_CLI_SUBCOMMANDS.includes(arg)) { + continue + } + // value of a space-separated flag (`--spec ./a.js`), not a positional + if (previous && previous.startsWith('-') && !previous.includes('=')) { + continue + } + if (!SUPPORTED_WDIO_CONFIG_EXTENSIONS.includes(path.extname(arg))) { + continue + } + + const resolved = resolveCandidate(arg) + if (resolved) { + return resolved + } + } + return undefined +} + +/** + * Resolve the absolute path of the user's wdio config file. + * + * WDIO keeps the real path in `ConfigParser`'s private `#configFilePath` field, which no + * service can reach, so this walks a ladder of fallbacks — first rung that points at a + * file on disk wins: + * + * 1. BROWSERSTACK_WDIO_CONFIG_FILE_PATH — explicit override / support escape hatch + * 2. config['config-path'] — yargs' kebab alias of the `run ` + * positional, which survives into the merged + * config object (v8 and v9 alike) + * 3. process.argv positional — `wdio ` without the `run` subcommand + * 4. config._[0] — same positional as seen by yargs + * 5. rootDir + wdio.conf. — no-arg `wdio`, and programmatic `new Launcher()` + * 6. cwd + wdio.conf. — when the user overrides `rootDir` in their config + * 7. single `*.conf.` in rootDir/cwd — unambiguous custom filenames only + */ +export function resolveWdioConfigPath(config?: Options.Testrunner): ConfigPathResolution { + const configRecord = (config || {}) as Record + + const fromEnv = resolveCandidate(process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH]) + if (fromEnv) { + return { configPath: fromEnv, strategy: 'env_override' } + } + + const fromConfigPath = resolveCandidate(configRecord['config-path']) + if (fromConfigPath) { + return { configPath: fromConfigPath, strategy: 'cli_config_path' } + } + + const fromArgv = scanArgvForConfig(process.argv.slice(2)) + if (fromArgv) { + return { configPath: fromArgv, strategy: 'argv_positional' } + } + + const positionals = Array.isArray(configRecord._) ? configRecord._ as unknown[] : [] + for (const positional of positionals) { + const resolved = resolveCandidate(positional) + if (resolved && SUPPORTED_WDIO_CONFIG_EXTENSIONS.includes(path.extname(resolved))) { + return { configPath: resolved, strategy: 'config_positional' } + } + } + + // `rootDir` defaults to dirname(configFile) but the user can override it in their + // config, so it is a fallback and never the source of truth — try cwd as well. + const rootDir = typeof configRecord.rootDir === 'string' ? configRecord.rootDir : undefined + const searchDirs = [rootDir, process.cwd()].filter((dir): dir is string => Boolean(dir)) + const uniqueDirs = Array.from(new Set(searchDirs)) + + for (const dir of uniqueDirs) { + const probed = probeConfigBasename(dir, DEFAULT_WDIO_CONFIG_BASENAME) + if (probed) { + return { configPath: probed, strategy: dir === rootDir ? 'root_dir_default' : 'cwd_default' } + } + } + + let sawAmbiguous = false + for (const dir of uniqueDirs) { + const { match, ambiguous } = scanForSingleConfig(dir) + if (match) { + return { configPath: match, strategy: 'single_conf_scan' } + } + sawAmbiguous = sawAmbiguous || Boolean(ambiguous) + } + + return { reason: sawAmbiguous ? 'config_ambiguous' : 'config_not_found' } +} + +/** + * Resolve once, as early as possible, and publish the answer on the environment so the + * upload path (and any worker) reads the SAME value instead of re-deriving it from cwd. + * + * Re-resolving at archive time is precisely the bug SDK-5993 fixed in the Node SDK: the + * archive step read `cwd/browserstack.yml` while startup had resolved a different path, + * silently dropping the config for every monorepo / subdir CI run. + */ +export function initWdioConfigPath(config?: Options.Testrunner): ConfigPathResolution { + try { + const alreadyResolved = process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] + if (alreadyResolved) { + // Report the rung that ORIGINALLY answered, not the env var this function + // itself wrote — otherwise every run reports `env_override` and the metric + // can never tell us how often the fallbacks are carrying customers. + return { + configPath: alreadyResolved, + strategy: process.env[BROWSERSTACK_WDIO_CONFIG_STRATEGY] || 'env_override' + } + } + + const resolution = resolveWdioConfigPath(config) + if (resolution.configPath) { + process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] = resolution.configPath + if (resolution.strategy) { + process.env[BROWSERSTACK_WDIO_CONFIG_STRATEGY] = resolution.strategy + } + // Path first: BStackLogger scrubs any `<...>key:`/`<...>user:` prefixed value, so a + // strategy name ending in `key`/`user` right before the path would redact the path. + BStackLogger.debug(`Resolved wdio config file ${resolution.configPath} for auto-capture (strategy ${resolution.strategy})`) + } else { + BStackLogger.debug(`Could not resolve wdio config file for auto-capture: ${resolution.reason}`) + } + return resolution + } catch (error) { + BStackLogger.debug(`Error while resolving wdio config file: ${error}`) + return { reason: 'config_resolve_exception' } + } +} + +/** + * Opt-out for auto-captured logs. Service option first, env var as the CI escape hatch + * (customers cannot always edit a committed config). Name matches the Node SDK's + * `disableAutoCaptureLogs` so the flag means the same thing across BrowserStack SDKs. + */ +export function isAutoCaptureLogsDisabled(options?: { disableAutoCaptureLogs?: boolean }): boolean { + if (options?.disableAutoCaptureLogs === true) { + return true + } + return String(process.env[BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS] || '').toLowerCase() === 'true' +} + +/** + * Mirror the service option onto the environment so the opt-out survives into the + * DETACHED cleanup process, which gets no options object. + * + * Without this the opt-out is worse than useless: skipping the upload leaves + * `logsUploaded` false, which is exactly the condition that arms the exit-time + * `--uploadLogs` rescue — so every opted-out run had its config read and POSTed by the + * cleanup child. Returns whether auto-capture is disabled. + */ +export function publishAutoCaptureDisabled(options?: { disableAutoCaptureLogs?: boolean }): boolean { + const disabled = isAutoCaptureLogsDisabled(options) + if (disabled) { + process.env[BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS] = 'true' + } + return disabled +} + +/** + * Line-level credential scrub, ported from the Node SDK's `redactSensitiveContent`. + * + * Any line mentioning a sensitive key collapses to `: [REDACTED]`. Word boundaries + * keep `hotkey` / `keyword` from tripping the bare `key` entry that WDIO's top-level + * credential options force us to carry. `.` is intentionally NOT part of the boundary + * class so `bstackOptions.accessKey = '...'` still matches. + */ +export function redactSensitiveContent(text: string): string { + if (!text) { + return text + } + + const escapeRegex = (value: string) => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + // longest first: alternation returns the first match, not the longest one + const keys = [...REDACTED_KEYS] + .sort((a, b) => b.length - a.length) + .map(escapeRegex) + .join('|') + const redactRegex = new RegExp(`^.*?(? { + try { + const size = fs.statSync(filePath).size + if (size > MAX_CAPTURED_CONFIG_FILE_BYTES) { + return { reason: `${path.basename(filePath)}: too_large (${size} bytes)` } + } + return { content: fs.readFileSync(filePath, 'utf8') } + } catch (error) { + return { reason: `${path.basename(filePath)}: ${(error as Error)?.message || String(error)}` } + } +} + +/** + * Resolve a relative import specifier against the importing file. + * + * Handles the TypeScript-ESM convention where `./shared.conf.js` on disk is actually + * `./shared.conf.ts`, plus extension-less and directory (`/index.*`) specifiers. + */ +const resolveRelativeImport = (specifier: string, fromFile: string): string | undefined => { + const base = path.resolve(path.dirname(fromFile), specifier) + + if (isReadableFile(base) && SUPPORTED_WDIO_CONFIG_EXTENSIONS.includes(path.extname(base))) { + return base + } + + const withoutExt = SUPPORTED_WDIO_CONFIG_EXTENSIONS.includes(path.extname(base)) + ? base.slice(0, -path.extname(base).length) + : base + + for (const ext of SUPPORTED_WDIO_CONFIG_EXTENSIONS) { + const candidate = `${withoutExt}${ext}` + if (isReadableFile(candidate)) { + return candidate + } + } + for (const ext of SUPPORTED_WDIO_CONFIG_EXTENSIONS) { + const candidate = path.join(base, `index${ext}`) + if (isReadableFile(candidate)) { + return candidate + } + } + return undefined +} + +/** + * Collect local files a config pulls in (`wdio.shared.conf.ts` style splits), so a + * captured config is not just a two-line file that spreads a base config we never see. + * + * Only RELATIVE specifiers are followed — bare specifiers are npm packages, never the + * customer's own config. Depth and count are capped so this can never walk a source tree. + */ +const collectLocalImports = (entryPath: string, entryContent: string, budget: number): string[] => { + const found: string[] = [] + const seen = new Set([entryPath]) + let frontier: Array<{ filePath: string, content: string }> = [{ filePath: entryPath, content: entryContent }] + + for (let depth = 0; depth < CAPTURE_CONFIG_IMPORT_DEPTH && found.length < budget; depth++) { + const next: Array<{ filePath: string, content: string }> = [] + + for (const { filePath, content } of frontier) { + // `from './x'`, `import './x'`, `import('./x')`, `require('./x')` + const importRegex = /(?:from|import|require)\s*\(?\s*['"](\.[^'"]*)['"]/g + let match: RegExpExecArray | null + + while ((match = importRegex.exec(content)) !== null) { + if (found.length >= budget) { + break + } + const resolved = resolveRelativeImport(match[1], filePath) + if (!resolved || seen.has(resolved) || resolved.includes(`${path.sep}node_modules${path.sep}`)) { + continue + } + seen.add(resolved) + + const { content: importedContent } = readCappedFile(resolved) + if (importedContent === undefined) { + continue + } + found.push(resolved) + next.push({ filePath: resolved, content: importedContent }) + } + } + + if (next.length === 0) { + break + } + frontier = next + } + + return found +} + +/** + * Give every archive entry a unique name. Two configs can share a basename + * (`configs/wdio.conf.ts` + `shared/wdio.conf.ts`); without this the second silently + * overwrites the first, since archive entries are keyed by basename. + */ +const uniqueEntryName = (filePath: string, taken: Set): string => { + const base = path.basename(filePath) + if (!taken.has(base)) { + taken.add(base) + return base + } + + const ext = path.extname(base) + const stem = ext ? base.slice(0, -ext.length) : base + for (let i = 1; i < MAX_CAPTURED_CONFIG_FILES + 2; i++) { + const candidate = `${stem}.${i}${ext}` + if (!taken.has(candidate)) { + taken.add(candidate) + return candidate + } + } + taken.add(base) + return base +} + +/** + * Build the redacted config entries added to the auto-captured log archive. + * + * Best effort by contract: any failure returns what was gathered so far and a reason + * string for the SDK_UPLOAD_LOGS event. It must never throw — a debug artifact is never + * worth failing a customer's test run over. + */ +export function collectConfigFilesForUpload(config?: Options.Testrunner): { files: CapturedFile[], failures: string[], strategy?: string } { + const failures: string[] = [] + const files: CapturedFile[] = [] + const takenNames = new Set() + + try { + const resolution = initWdioConfigPath(config) + if (!resolution.configPath) { + failures.push(resolution.reason || 'config_not_found') + return { files, failures } + } + + const { content, reason } = readCappedFile(resolution.configPath) + if (content === undefined) { + failures.push(reason || 'config_read_failed') + return { files, failures, strategy: resolution.strategy } + } + + files.push({ + name: uniqueEntryName(resolution.configPath, takenNames), + sourcePath: resolution.configPath, + content: redactSensitiveContent(content) + }) + + const remaining = MAX_CAPTURED_CONFIG_FILES - files.length + if (remaining > 0) { + for (const importedPath of collectLocalImports(resolution.configPath, content, remaining)) { + const imported = readCappedFile(importedPath) + if (imported.content === undefined) { + if (imported.reason) { + failures.push(imported.reason) + } + continue + } + files.push({ + name: uniqueEntryName(importedPath, takenNames), + sourcePath: importedPath, + content: redactSensitiveContent(imported.content) + }) + } + } + + return { files, failures, strategy: resolution.strategy } + } catch (error) { + failures.push(`config_capture_exception: ${(error as Error)?.message || String(error)}`) + return { files, failures } + } +} + +/** + * Walk up from `startDir` looking for `fileName`, stopping at the filesystem root or + * after MAX_PACKAGE_JSON_WALK_UP levels. + */ +const findUpwards = (startDir: string, fileName: string): string | undefined => { + let current = startDir + for (let depth = 0; depth <= MAX_PACKAGE_JSON_WALK_UP; depth++) { + const candidate = path.join(current, fileName) + if (isReadableFile(candidate)) { + return candidate + } + const parent = path.dirname(current) + if (parent === current) { + break + } + current = parent + } + return undefined +} + +/** + * `package.json` for the project the config belongs to — framework and service versions + * are the first thing triage needs, and the archive carried neither before. + * + * Walks UP from the config's directory, because `configs/wdio.conf.ts` (a very common + * layout) puts the manifest one or more levels above the config, not beside it. + * Archived verbatim: it is a manifest, not a secret store. + */ +export function findPackageJsonForUpload(): string | undefined { + const configPath = process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] + const startDirs = [configPath ? path.dirname(configPath) : undefined, process.cwd()] + .filter((dir): dir is string => Boolean(dir)) + + for (const dir of Array.from(new Set(startDirs))) { + const found = findUpwards(dir, 'package.json') + if (found) { + return found + } + } + return undefined +} diff --git a/packages/browserstack-service/src/constants.ts b/packages/browserstack-service/src/constants.ts index 71e2f49..f489b4a 100644 --- a/packages/browserstack-service/src/constants.ts +++ b/packages/browserstack-service/src/constants.ts @@ -52,6 +52,46 @@ export const UPLOAD_LOGS_ENDPOINT = 'client-logs/upload' export const PERCY_LOGS_FILE = 'logs/percy.log' +/** + * Auto-capture of the user's wdio config file (SDK-7250). + */ + +/* Absolute path of the resolved wdio config, published once so the upload path never re-derives it */ +export const BROWSERSTACK_WDIO_CONFIG_FILE_PATH = 'BROWSERSTACK_WDIO_CONFIG_FILE_PATH' +export const BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS = 'BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS' +/* Which ladder rung resolved the config, kept so the upload path reports the TRUE rung + instead of re-reading its own env var and always saying 'env_override' */ +export const BROWSERSTACK_WDIO_CONFIG_STRATEGY = 'BROWSERSTACK_WDIO_CONFIG_STRATEGY' + +/* Mirrors create-wdio's SUPPORTED_CONFIG_FILE_EXTENSION (identical in wdio v8 and v9) */ +export const SUPPORTED_WDIO_CONFIG_EXTENSIONS = ['.js', '.ts', '.mjs', '.mts', '.cjs', '.cts'] +export const DEFAULT_WDIO_CONFIG_BASENAME = 'wdio.conf' +/* `wdio ` verbs that must never be mistaken for the config positional */ +export const WDIO_CLI_SUBCOMMANDS = ['run', 'install', 'repl', 'config'] + +/* Configs are kilobytes; the cap only exists so a mislabelled path cannot bloat the archive */ +export const MAX_CAPTURED_CONFIG_FILE_BYTES = 1024 * 1024 +export const MAX_CAPTURED_CONFIG_FILES = 6 +/* How far to follow relative imports out of the entry config (1 = direct imports only) */ +export const CAPTURE_CONFIG_IMPORT_DEPTH = 1 +/* How far to walk up from the config dir looking for the project's package.json */ +export const MAX_PACKAGE_JSON_WALK_UP = 5 + +/** + * Keys whose line is scrubbed before a config file enters the archive. + * `user` / `key` are WDIO's own top-level credential options, hence the bare entries. + */ +export const REDACTED_KEYS = [ + 'user', 'key', + 'userName', 'accessKey', + 'browserstack.user', 'browserstack.key', + 'browserstack.userName', 'browserstack.accessKey', + 'password', 'proxyPassword', 'proxyUser', 'proxyPass', + 'localProxyUser', 'localProxyPass', 'proxyUrl', + 'authToken', 'apiKey', 'accessToken', 'secret', 'token', + 'customVariables', 'user_data', 'httpProxy', 'httpsProxy' +] + export const PERCY_DOM_CHANGING_COMMANDS_ENDPOINTS = [ '/session/:sessionId/url', '/session/:sessionId/forward', diff --git a/packages/browserstack-service/src/exitHandler.ts b/packages/browserstack-service/src/exitHandler.ts index 09e6783..c33438b 100644 --- a/packages/browserstack-service/src/exitHandler.ts +++ b/packages/browserstack-service/src/exitHandler.ts @@ -6,6 +6,7 @@ import { fileURLToPath } from 'node:url' import PerformanceTester from './instrumentation/performance/performance-tester.js' import TestOpsConfig from './testOps/testOpsConfig.js' import { BStackLogger } from './bstackLogger.js' +import { isAutoCaptureLogsDisabled } from './configCapture.js' import { BrowserstackCLI } from './cli/index.js' import { BROWSERSTACK_TESTHUB_JWT, BROWSERSTACK_TESTHUB_UUID, BROWSERSTACK_KILL_SIGNAL } from './constants.js' @@ -108,8 +109,10 @@ export function shouldCallCleanup(config: BrowserStackConfig, isCLIEnabled = fal // A signal-terminated run never reaches onComplete's log upload, leaving the // build with no SDK-log object — rescue it from the detached cleanup process. + // Opting out leaves logsUploaded false, so without this guard the rescue below fires + // on EVERY opted-out run and uploads exactly what the user opted out of. const clientBuildUuid = process.env[BROWSERSTACK_TESTHUB_UUID] || config.sdkRunID - if (!config.logsUploaded && config.userName && config.accessKey && clientBuildUuid) { + if (!isAutoCaptureLogsDisabled() && !config.logsUploaded && config.userName && config.accessKey && clientBuildUuid) { args.push('--uploadLogs', clientBuildUuid) } diff --git a/packages/browserstack-service/src/launcher.ts b/packages/browserstack-service/src/launcher.ts index 00fb918..a6eecfa 100644 --- a/packages/browserstack-service/src/launcher.ts +++ b/packages/browserstack-service/src/launcher.ts @@ -50,6 +50,7 @@ import { validateSkipAppOverride } from './util.js' import CrashReporter from './crash-reporter.js' +import { initWdioConfigPath, isAutoCaptureLogsDisabled, publishAutoCaptureDisabled } from './configCapture.js' import { finalizeOrphanedRuns } from './testOps/openRunsJournal.js' import { BStackLogger } from './bstackLogger.js' import { PercyLogger } from './Percy/PercyLogger.js' @@ -250,6 +251,15 @@ export default class BrowserstackLauncherService implements Services.ServiceInst async onPrepare (config: Options.Testrunner, capabilities: Capabilities.TestrunnerCapabilities | WebdriverIO.Capabilities) { PerformanceTester.start(PERFORMANCE_SDK_EVENTS.FRAMEWORK_EVENTS.INIT) + // Resolve the user's wdio config path ONCE, here, while the freshly parsed config + // still carries the CLI's `config-path` positional, and publish it on the env for + // the upload path. Re-deriving it at archive time from cwd is the exact bug + // SDK-5993 fixed in the Node SDK (silently dropped the config on every monorepo / + // subdir CI run). Best-effort: never blocks the run. + if (!publishAutoCaptureDisabled(this._options)) { + initWdioConfigPath(config) + } + // skipAppOverride: emit the fixed warning once + handle the 3 edge cases before anything // else. Runs once here in the launcher (main process). Edge-2 (explicit false + no app) is a // deliberate pre-session config error, surfaced as SevereServiceError so the run aborts cleanly. @@ -838,7 +848,12 @@ export default class BrowserstackLauncherService implements Services.ServiceInst // return path (no creds, archive failure, upload no-response, exception), so // measureWrapper is no longer needed here. const clientBuildUuid = this._getClientBuildUuid() - const response = await uploadLogs(getBrowserStackUser(this._config), getBrowserStackKey(this._config), clientBuildUuid) + const response = await uploadLogs( + getBrowserStackUser(this._config), + getBrowserStackKey(this._config), + clientBuildUuid, + { disableAutoCaptureLogs: isAutoCaptureLogsDisabled(this._options), config: this._config } + ) // Treat a truthy response carrying a non-success status as a server-side // rejection, not a delivery — a delivered upload must not be repeated by // the exit-time cleanup rescue; failed/skipped uploads stay eligible for it. diff --git a/packages/browserstack-service/src/types.ts b/packages/browserstack-service/src/types.ts index d01b9d8..b4e50ae 100644 --- a/packages/browserstack-service/src/types.ts +++ b/packages/browserstack-service/src/types.ts @@ -103,6 +103,15 @@ export interface BrowserstackConfig { * Currently supports testPlanId. */ testManagementOptions?: TestManagementOptions; + /** + * By default the service uploads its own debug logs, your `package.json` and a + * credential-redacted copy of your wdio config file at the end of a run, so + * BrowserStack support can debug issues without asking you to reproduce them. + * Set this to true to disable that upload entirely. + * Can also be set with the `BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS=true` env var. + * @default false + */ + disableAutoCaptureLogs?: boolean; /** * Set this to true to enable BrowserStack Percy which will take screenshots * and snapshots for your tests run on Browserstack diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index 972ce45..778ecc5 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -48,6 +48,7 @@ import { } from './constants.js' import CrashReporter from './crash-reporter.js' import { BStackLogger } from './bstackLogger.js' +import { collectConfigFilesForUpload, findPackageJsonForUpload, isAutoCaptureLogsDisabled } from './configCapture.js' import UsageStats from './testOps/usageStats.js' import TestOpsConfig from './testOps/testOpsConfig.js' import type { StartBinSessionResponse } from './grpc/index.js' @@ -1516,7 +1517,14 @@ export function getFailureObject(error: string|Error) { export const sleep = (ms = 100) => new Promise((resolve) => setTimeout(resolve, ms)) -export async function uploadLogs(user: string | undefined, key: string | undefined, clientBuildUuid: string) { +export interface UploadLogsOptions { + /* opt-out, mirroring the Node SDK's `disableAutoCaptureLogs` */ + disableAutoCaptureLogs?: boolean + /* parsed wdio config, used to locate the user's config file */ + config?: Options.Testrunner +} + +export async function uploadLogs(user: string | undefined, key: string | undefined, clientBuildUuid: string, options: UploadLogsOptions = {}) { // Manual instrumentation: tag every return path on the SDK_UPLOAD_LOGS event so // the metric identifies the specific reason logs were not uploaded (no creds, // per-file copy failure, upload no-response, exception). measureWrapper would @@ -1524,9 +1532,24 @@ export async function uploadLogs(user: string | undefined, key: string | undefin const eventName = PERFORMANCE_SDK_EVENTS.EVENTS.SDK_UPLOAD_LOGS let success = true let failure: string | undefined + // Staging dir for the archive. Created per run: the previous fixed `tmpdir()/logs.tar` + // names meant two concurrent wdio runs on one CI host clobbered and unlinked each + // other's archives, and a source file that already lived in tmpdir was copied onto + // itself and then deleted by cleanup. + let stagingDir: string | undefined PerformanceTester.start(eventName) try { + // isAutoCaptureLogsDisabled (not options.disableAutoCaptureLogs) so the detached + // cleanup process — which calls this with no options — still honours the opt-out + // via the env var the launcher publishes. + if (isAutoCaptureLogsDisabled(options)) { + success = false + failure = 'skipped: disableAutoCaptureLogs=true' + BStackLogger.debug('Skipping log upload, auto-capture is disabled') + return + } + if (!user || !key) { success = false failure = 'skipped: missing_credentials' @@ -1534,29 +1557,78 @@ export async function uploadLogs(user: string | undefined, key: string | undefin return } - const tmpDir = tmpdir() + stagingDir = fs.mkdtempSync(path.join(tmpdir(), 'bstack-wdio-logs-')) + const tmpDir = stagingDir const tarPath = path.join(tmpDir, 'logs.tar') const tarGzPath = path.join(tmpDir, 'logs.tar.gz') + // Archive entries are keyed by basename, so two sources sharing one basename would + // silently overwrite each other — reachable now that user-supplied config paths + // (e.g. configs/wdio.conf.ts + shared/wdio.conf.ts) join the archive. + const takenNames = new Set(['logs.tar', 'logs.tar.gz']) + const uniqueName = (filePath: string): string => { + const base = path.basename(filePath) + if (!takenNames.has(base)) { + takenNames.add(base) + return base + } + const ext = path.extname(base) + const stem = ext ? base.slice(0, -ext.length) : base + let index = 1 + let candidate = `${stem}.${index}${ext}` + while (takenNames.has(candidate)) { + index++ + candidate = `${stem}.${index}${ext}` + } + takenNames.add(candidate) + return candidate + } + const filesToArchive = [ BStackLogger.logFilePath, CLI_DEBUG_LOGS_FILE, - ].filter(f => fs.existsSync(f)) + // framework/service versions — first thing triage needs, and the archive + // carried neither before (the Node SDK has shipped package.json for years) + findPackageJsonForUpload(), + ].filter((f): f is string => Boolean(f) && fs.existsSync(f as string)) const copiedFileNames: string[] = [] const archiveAddFailures: string[] = [] for (const f of filesToArchive) { try { - const dest = path.join(tmpDir, path.basename(f)) - fs.copyFileSync(f, dest) - copiedFileNames.push(path.basename(f)) + const entryName = uniqueName(f) + fs.copyFileSync(f, path.join(tmpDir, entryName)) + copiedFileNames.push(entryName) } catch (copyErr) { const msg = (copyErr as Error)?.message || String(copyErr) archiveAddFailures.push(`${path.basename(f)}: ${msg}`) } } - if (archiveAddFailures.length > 0 && failure === undefined) { + // SDK-7250: the user's wdio config (and the local files it imports), credential-scrubbed. + // Soft-failure by design — a config we cannot read or locate must never stop the + // log upload; the reason is recorded on the event instead. + const { files: configFiles, failures: configFailures, strategy } = collectConfigFilesForUpload(options.config) + for (const configFile of configFiles) { + try { + const entryName = uniqueName(configFile.name) + fs.writeFileSync(path.join(tmpDir, entryName), configFile.content) + copiedFileNames.push(entryName) + } catch (writeErr) { + const msg = (writeErr as Error)?.message || String(writeErr) + configFailures.push(`${configFile.name}: ${msg}`) + } + } + if (configFiles.length > 0) { + BStackLogger.debug(`Auto-captured ${configFiles.length} config file(s) via ${strategy}: ${configFiles.map(f => f.name).join(', ')}`) + } + if (configFailures.length > 0 && failure === undefined) { + // Warning only — `success` stays true so a missing config never reads as a + // failed log upload (same contract as the Node SDK's `redaction_failed`). + failure = `config_capture: ${configFailures.join('; ')}`.substring(0, 300) + } + + if (archiveAddFailures.length > 0) { success = false failure = `archive_add_failed [${archiveAddFailures.length}]: ${archiveAddFailures.join('; ')}`.substring(0, 300) } @@ -1605,14 +1677,7 @@ export async function uploadLogs(user: string | undefined, key: string | undefin 'POST', UPLOAD_LOGS_ENDPOINT, requestOptions, APIUtils.UPLOAD_LOGS_ADDRESS ) - fs.unlinkSync(tarPath) - fs.unlinkSync(tarGzPath) - for (const f of copiedFileNames) { - const filePath = path.join(tmpDir, f) - if (fs.existsSync(filePath)) { - fs.unlinkSync(filePath) - } - } + // Staging dir (archive + every copied/redacted entry) is removed in `finally`. // Delete the SDK CLI log file after upload if (fs.existsSync(CLI_DEBUG_LOGS_FILE)) { @@ -1638,6 +1703,13 @@ export async function uploadLogs(user: string | undefined, key: string | undefin BStackLogger.error(`Error while uploading logs: ${getErrorString(error)}`) return null } finally { + if (stagingDir) { + try { + fs.rmSync(stagingDir, { recursive: true, force: true }) + } catch (cleanupErr) { + BStackLogger.debug(`Failed to clean up log staging dir ${stagingDir}: ${getErrorString(cleanupErr)}`) + } + } PerformanceTester.end(eventName, success, failure) } } diff --git a/packages/browserstack-service/tests/configCapture.test.ts b/packages/browserstack-service/tests/configCapture.test.ts new file mode 100644 index 0000000..1656e4e --- /dev/null +++ b/packages/browserstack-service/tests/configCapture.test.ts @@ -0,0 +1,354 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' + +import { + collectConfigFilesForUpload, + findPackageJsonForUpload, + initWdioConfigPath, + isAutoCaptureLogsDisabled, + redactSensitiveContent, + resolveWdioConfigPath +} from '../src/configCapture.js' +import { + BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS, + BROWSERSTACK_WDIO_CONFIG_FILE_PATH, + BROWSERSTACK_WDIO_CONFIG_STRATEGY +} from '../src/constants.js' + +const CONFIG_SRC = ` +import { shared } from './shared.conf.js' + +export const config = { + ...shared, + user: 'my-real-username', + key: 'my-real-access-key', + hostname: 'hub.browserstack.com', + capabilities: [{ + 'bstack:options': { userName: 'someone', accessKey: 'sk_live_abcdef' } + }], + services: [['browserstack', { accessibility: true }]] +} +` + +let tmpRoot: string +let cwdSpy: ReturnType +const originalArgv = process.argv + +const useCwd = (dir: string) => { + cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(dir) +} + +beforeEach(() => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'bstack-cfg-test-')) + delete process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] + delete process.env[BROWSERSTACK_WDIO_CONFIG_STRATEGY] + delete process.env[BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS] + process.argv = ['node', 'wdio'] + useCwd(tmpRoot) +}) + +afterEach(() => { + cwdSpy?.mockRestore() + process.argv = originalArgv + delete process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] + delete process.env[BROWSERSTACK_WDIO_CONFIG_STRATEGY] + delete process.env[BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS] + fs.rmSync(tmpRoot, { recursive: true, force: true }) +}) + +const write = (relativePath: string, content = 'export const config = {}') => { + const absolute = path.join(tmpRoot, relativePath) + fs.mkdirSync(path.dirname(absolute), { recursive: true }) + fs.writeFileSync(absolute, content) + return absolute +} + +describe('resolveWdioConfigPath', () => { + it('prefers the BROWSERSTACK_WDIO_CONFIG_FILE_PATH override', () => { + const override = write('somewhere/custom.conf.ts') + write('wdio.conf.js') + process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] = override + + expect(resolveWdioConfigPath({} as never)).toEqual({ configPath: override, strategy: 'env_override' }) + }) + + it('resolves a relative `config-path` against cwd (wdio run ./x.conf.ts)', () => { + const expected = write('configs/wdio.bstack.conf.ts') + + expect(resolveWdioConfigPath({ 'config-path': './configs/wdio.bstack.conf.ts' } as never)) + .toEqual({ configPath: expected, strategy: 'cli_config_path' }) + }) + + it('resolves an absolute `config-path`', () => { + const expected = write('wdio.conf.ts') + + expect(resolveWdioConfigPath({ 'config-path': expected } as never)) + .toEqual({ configPath: expected, strategy: 'cli_config_path' }) + }) + + it('ignores a `config-path` that no longer exists and falls through', () => { + const expected = write('wdio.conf.js') + + expect(resolveWdioConfigPath({ 'config-path': './deleted.conf.ts' } as never)) + .toEqual({ configPath: expected, strategy: 'cwd_default' }) + }) + + it('falls back to the argv positional for the bare `wdio ` form', () => { + const expected = write('custom.conf.mjs') + process.argv = ['node', 'wdio', './custom.conf.mjs'] + + expect(resolveWdioConfigPath({} as never)) + .toEqual({ configPath: expected, strategy: 'argv_positional' }) + }) + + it('does not mistake a --spec value for the config positional', () => { + const expected = write('wdio.conf.ts') + write('test/login.e2e.js') + process.argv = ['node', 'wdio', 'run', './wdio.conf.ts', '--spec', './test/login.e2e.js'] + + expect(resolveWdioConfigPath({} as never)) + .toEqual({ configPath: expected, strategy: 'argv_positional' }) + }) + + it('skips the `run` subcommand when scanning argv', () => { + write('run') + const expected = write('wdio.conf.cts') + process.argv = ['node', 'wdio', 'run', './wdio.conf.cts'] + + expect(resolveWdioConfigPath({} as never).configPath).toBe(expected) + }) + + it('probes rootDir for wdio.conf with every supported extension', () => { + const nested = path.join(tmpRoot, 'nested') + fs.mkdirSync(nested) + const expected = write('nested/wdio.conf.mts') + + expect(resolveWdioConfigPath({ rootDir: nested } as never)) + .toEqual({ configPath: expected, strategy: 'root_dir_default' }) + }) + + it('falls back to cwd when the user overrides rootDir', () => { + const expected = write('wdio.conf.js') + + expect(resolveWdioConfigPath({ rootDir: '/definitely/not/here' } as never)) + .toEqual({ configPath: expected, strategy: 'cwd_default' }) + }) + + it('accepts a single custom *.conf.* file as unambiguous', () => { + const expected = write('e2e.conf.ts') + + expect(resolveWdioConfigPath({} as never)) + .toEqual({ configPath: expected, strategy: 'single_conf_scan' }) + }) + + it('captures nothing when several custom configs are present', () => { + write('android.conf.ts') + write('ios.conf.ts') + + expect(resolveWdioConfigPath({} as never)).toEqual({ reason: 'config_ambiguous' }) + }) + + it('reports config_not_found on an empty project', () => { + expect(resolveWdioConfigPath({} as never)).toEqual({ reason: 'config_not_found' }) + }) +}) + +describe('initWdioConfigPath', () => { + it('publishes the resolved path on the environment', () => { + const expected = write('wdio.conf.js') + + initWdioConfigPath({} as never) + + expect(process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH]).toBe(expected) + }) + + it('reports the original rung on later calls, not env_override', () => { + write('configs/wdio.custom.conf.ts') + + const first = initWdioConfigPath({ 'config-path': './configs/wdio.custom.conf.ts' } as never) + const second = initWdioConfigPath({} as never) + + expect(first.strategy).toBe('cli_config_path') + expect(second.strategy).toBe('cli_config_path') + expect(second.configPath).toBe(first.configPath) + }) + + it('reports env_override when the user set the env var themselves', () => { + process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] = write('wdio.conf.js') + + expect(initWdioConfigPath({} as never).strategy).toBe('env_override') + }) + + it('leaves the env untouched when nothing resolves', () => { + initWdioConfigPath({} as never) + + expect(process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH]).toBeUndefined() + }) +}) + +describe('redactSensitiveContent', () => { + it('scrubs wdio top-level credentials and bstack:options credentials', () => { + const redacted = redactSensitiveContent(CONFIG_SRC) + + expect(redacted).not.toContain('my-real-username') + expect(redacted).not.toContain('my-real-access-key') + expect(redacted).not.toContain('sk_live_abcdef') + expect(redacted).not.toContain('someone') + }) + + it('keeps non-credential config intact', () => { + const redacted = redactSensitiveContent(CONFIG_SRC) + + expect(redacted).toContain('accessibility: true') + expect(redacted).toContain('hub.browserstack.com') + }) + + it('does not over-redact identifiers that merely contain key/user', () => { + const redacted = redactSensitiveContent([ + 'const hotkey = "ctrl+a"', + 'const keyword = "search"', + 'const username_suffix_thing = 1', + 'monkeypatch()' + ].join('\n')) + + expect(redacted).toContain('ctrl+a') + expect(redacted).toContain('search') + expect(redacted).toContain('monkeypatch()') + }) + + it('scrubs dotted property assignment (bstackOptions.accessKey = ...)', () => { + expect(redactSensitiveContent('bstackOptions.accessKey = "leaked-key"')) + .not.toContain('leaked-key') + }) + + it('returns falsy input unchanged', () => { + expect(redactSensitiveContent('')).toBe('') + }) +}) + +describe('collectConfigFilesForUpload', () => { + it('captures the entry config, redacted', () => { + write('wdio.conf.ts', CONFIG_SRC) + write('shared.conf.ts', 'export const shared = { maxInstances: 5 }') + + const { files, failures } = collectConfigFilesForUpload({} as never) + + expect(failures).toEqual([]) + const entry = files.find((f) => f.name === 'wdio.conf.ts') + expect(entry).toBeDefined() + expect(entry!.content).not.toContain('my-real-access-key') + expect(entry!.content).toContain('accessibility: true') + }) + + it('follows relative imports, resolving a .js specifier to the .ts file on disk', () => { + write('wdio.conf.ts', CONFIG_SRC) + write('shared.conf.ts', 'export const shared = { maxInstances: 5 }') + + const { files } = collectConfigFilesForUpload({} as never) + + expect(files.map((f) => f.name).sort()).toEqual(['shared.conf.ts', 'wdio.conf.ts']) + expect(files.find((f) => f.name === 'shared.conf.ts')!.content).toContain('maxInstances: 5') + }) + + it('redacts imported config files too', () => { + write('wdio.conf.ts', 'import "./creds.conf.ts"\nexport const config = {}') + write('creds.conf.ts', 'export const accessKey = "leaked-from-import"') + + const { files } = collectConfigFilesForUpload({} as never) + + expect(files.map((f) => f.content).join('\n')).not.toContain('leaked-from-import') + }) + + it('never follows bare (npm package) specifiers', () => { + write('wdio.conf.ts', 'import { x } from "@wdio/globals"\nimport y from "dotenv"\nexport const config = {}') + + const { files } = collectConfigFilesForUpload({} as never) + + expect(files.map((f) => f.name)).toEqual(['wdio.conf.ts']) + }) + + it('de-duplicates archive names when two captured files share a basename', () => { + write('wdio.conf.ts', 'import "./nested/wdio.conf.ts"\nexport const config = {}') + write('nested/wdio.conf.ts', 'export const config = {}') + + const { files } = collectConfigFilesForUpload({} as never) + + expect(files).toHaveLength(2) + expect(new Set(files.map((f) => f.name)).size).toBe(2) + }) + + it('reports a soft failure and captures nothing when no config is found', () => { + const { files, failures } = collectConfigFilesForUpload({} as never) + + expect(files).toEqual([]) + expect(failures).toEqual(['config_not_found']) + }) + + it('skips an oversized config instead of bloating the archive', () => { + write('wdio.conf.js', 'x'.repeat(1024 * 1024 + 10)) + + const { files, failures } = collectConfigFilesForUpload({} as never) + + expect(files).toEqual([]) + expect(failures[0]).toContain('too_large') + }) + + it('never throws on an unreadable config path', () => { + process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] = path.join(tmpRoot, 'gone.conf.ts') + + expect(() => collectConfigFilesForUpload({} as never)).not.toThrow() + }) +}) + +describe('findPackageJsonForUpload', () => { + it('prefers the package.json next to the resolved config', () => { + const expected = write('project/package.json', '{"name":"app"}') + process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] = write('project/wdio.conf.ts') + + expect(findPackageJsonForUpload()).toBe(expected) + }) + + it('falls back to cwd', () => { + const expected = write('package.json', '{"name":"app"}') + + expect(findPackageJsonForUpload()).toBe(expected) + }) + + it('walks up from a config kept in a subdirectory (configs/wdio.conf.ts)', () => { + const expected = write('package.json', '{"name":"app"}') + process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] = write('configs/env/wdio.conf.ts') + + expect(findPackageJsonForUpload()).toBe(expected) + }) + + it('picks the closest package.json when nested ones exist', () => { + write('package.json', '{"name":"monorepo-root"}') + const closest = write('packages/e2e/package.json', '{"name":"e2e"}') + process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] = write('packages/e2e/configs/wdio.conf.ts') + + expect(findPackageJsonForUpload()).toBe(closest) + }) + + it('returns undefined when there is none', () => { + expect(findPackageJsonForUpload()).toBeUndefined() + }) +}) + +describe('isAutoCaptureLogsDisabled', () => { + it('is off by default', () => { + expect(isAutoCaptureLogsDisabled({})).toBe(false) + expect(isAutoCaptureLogsDisabled(undefined)).toBe(false) + }) + + it('honours the service option', () => { + expect(isAutoCaptureLogsDisabled({ disableAutoCaptureLogs: true })).toBe(true) + }) + + it('honours the env var for CI where the config cannot be edited', () => { + process.env[BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS] = 'TRUE' + + expect(isAutoCaptureLogsDisabled({})).toBe(true) + }) +}) diff --git a/packages/browserstack-service/tests/uploadLogsArchive.test.ts b/packages/browserstack-service/tests/uploadLogsArchive.test.ts new file mode 100644 index 0000000..f24c3f6 --- /dev/null +++ b/packages/browserstack-service/tests/uploadLogsArchive.test.ts @@ -0,0 +1,158 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import zlib from 'node:zlib' +import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' +import { list } from 'tar' + +import { uploadLogs } from '../src/util.js' +import { BStackLogger } from '../src/bstackLogger.js' +import { BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS, BROWSERSTACK_WDIO_CONFIG_FILE_PATH } from '../src/constants.js' +import { _fetch as fetch } from '../src/fetchWrapper.js' + +vi.mock('../src/fetchWrapper.js', () => ({ _fetch: vi.fn() })) +vi.mock('@wdio/logger', () => import(path.join(process.cwd(), '__mocks__', '@wdio/logger'))) + +/** the gzipped tarball actually handed to the upload endpoint */ +let uploadedArchive: Buffer | undefined +let tmpProject: string +let cwdSpy: ReturnType +let originalLogFilePath: string + +const readArchiveEntries = async (gz: Buffer) => { + const workDir = fs.mkdtempSync(path.join(os.tmpdir(), 'bstack-untar-')) + const tarPath = path.join(workDir, 'logs.tar') + fs.writeFileSync(tarPath, zlib.gunzipSync(gz)) + + const entries: string[] = [] + await list({ file: tarPath, onentry: (e) => entries.push(String(e.path)) }) + fs.rmSync(workDir, { recursive: true, force: true }) + return entries +} + +beforeEach(() => { + uploadedArchive = undefined + tmpProject = fs.mkdtempSync(path.join(os.tmpdir(), 'bstack-upload-test-')) + cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(tmpProject) + originalLogFilePath = BStackLogger.logFilePath + BStackLogger.logFilePath = path.join(tmpProject, 'bstack-wdio-service.log') + fs.writeFileSync(BStackLogger.logFilePath, 'service log content') + delete process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] + delete process.env[BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS] + + vi.mocked(fetch).mockImplementation((async (_url: string, init: RequestInit) => { + // read the archive here: uploadLogs removes its staging dir right after + const body = init?.body as FormData + const blob = body?.get?.('data') as Blob | null + if (blob && typeof blob.arrayBuffer === 'function') { + uploadedArchive = Buffer.from(await blob.arrayBuffer()) + } + return Response.json({ status: 'success' }) + }) as never) +}) + +afterEach(() => { + cwdSpy.mockRestore() + BStackLogger.logFilePath = originalLogFilePath + delete process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] + delete process.env[BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS] + fs.rmSync(tmpProject, { recursive: true, force: true }) + vi.mocked(fetch).mockReset() +}) + +describe('uploadLogs archive contents (SDK-7250)', () => { + it('ships the wdio config, its local import and package.json alongside the service log', async () => { + fs.writeFileSync(path.join(tmpProject, 'package.json'), '{"name":"customer-app","version":"1.2.3"}') + fs.writeFileSync(path.join(tmpProject, 'shared.conf.js'), 'export const shared = { maxInstances: 7 }') + fs.writeFileSync(path.join(tmpProject, 'wdio.conf.js'), [ + 'import { shared } from "./shared.conf.js"', + 'export const config = {', + ' ...shared,', + " user: 'LEAKED_USER_VALUE',", + " key: 'LEAKED_KEY_VALUE',", + ' services: [["browserstack", { accessibility: true }]]', + '}' + ].join('\n')) + + await uploadLogs('some_user', 'some_key', 'some_uuid', {}) + + expect(fetch).toHaveBeenCalled() + expect(uploadedArchive).toBeDefined() + + const entries = await readArchiveEntries(uploadedArchive!) + expect(entries).toContain('wdio.conf.js') + expect(entries).toContain('shared.conf.js') + expect(entries).toContain('package.json') + expect(entries).toContain('bstack-wdio-service.log') + }) + + it('scrubs credentials out of the archived config', async () => { + fs.writeFileSync(path.join(tmpProject, 'wdio.conf.js'), [ + 'export const config = {', + " user: 'LEAKED_USER_VALUE',", + " key: 'LEAKED_KEY_VALUE',", + ' services: [["browserstack", { accessibility: true }]]', + '}' + ].join('\n')) + + await uploadLogs('some_user', 'some_key', 'some_uuid', {}) + + const raw = zlib.gunzipSync(uploadedArchive!).toString('binary') + expect(raw).not.toContain('LEAKED_USER_VALUE') + expect(raw).not.toContain('LEAKED_KEY_VALUE') + // non-credential config must survive, else the artifact is useless for triage + expect(raw).toContain('accessibility: true') + }) + + it('still uploads the logs when no config can be found', async () => { + await uploadLogs('some_user', 'some_key', 'some_uuid', {}) + + expect(fetch).toHaveBeenCalled() + const entries = await readArchiveEntries(uploadedArchive!) + expect(entries).toContain('bstack-wdio-service.log') + expect(entries).not.toContain('wdio.conf.js') + }) + + it('uploads nothing when disableAutoCaptureLogs is set', async () => { + fs.writeFileSync(path.join(tmpProject, 'wdio.conf.js'), 'export const config = {}') + + await uploadLogs('some_user', 'some_key', 'some_uuid', { disableAutoCaptureLogs: true }) + + expect(fetch).not.toHaveBeenCalled() + }) + + it('honours the opt-out env var when called with no options (cleanup process)', async () => { + // The detached cleanup rescue calls uploadLogs(user, key, uuid) with NO options. + // Before the fix it uploaded the config of every user who had opted out, because + // opting out leaves `logsUploaded` false, which is what arms the rescue. + fs.writeFileSync(path.join(tmpProject, 'wdio.conf.js'), 'export const config = {}') + process.env[BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS] = 'true' + + await uploadLogs('some_user', 'some_key', 'some_uuid') + + expect(fetch).not.toHaveBeenCalled() + }) + + it('leaves no staging directory behind', async () => { + fs.writeFileSync(path.join(tmpProject, 'wdio.conf.js'), 'export const config = {}') + const before = fs.readdirSync(os.tmpdir()).filter((e) => e.startsWith('bstack-wdio-logs-')) + + await uploadLogs('some_user', 'some_key', 'some_uuid', {}) + + const after = fs.readdirSync(os.tmpdir()).filter((e) => e.startsWith('bstack-wdio-logs-')) + expect(after).toEqual(before) + }) + + it('keeps concurrent runs from clobbering each other', async () => { + fs.writeFileSync(path.join(tmpProject, 'wdio.conf.js'), 'export const config = {}') + + const results = await Promise.all([ + uploadLogs('some_user', 'some_key', 'uuid-1', {}), + uploadLogs('some_user', 'some_key', 'uuid-2', {}), + uploadLogs('some_user', 'some_key', 'uuid-3', {}) + ]) + + expect(results.every((r) => r && r.status === 'success')).toBe(true) + expect(fetch).toHaveBeenCalledTimes(3) + }) +}) From aa16aa10a56c29dd841bc79d151ea1f4efd8e3d1 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:10:22 +0000 Subject: [PATCH 02/14] chore(changeset): auto-generate from PR template (minor) --- .changeset/pr-128.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/pr-128.md diff --git a/.changeset/pr-128.md b/.changeset/pr-128.md new file mode 100644 index 0000000..8499bbb --- /dev/null +++ b/.changeset/pr-128.md @@ -0,0 +1,6 @@ +--- +"@wdio/browserstack-service": minor +--- + +- The debug logs the service uploads at the end of a run now include a copy of your `wdio.conf` file (and the local config files it imports) with credentials removed, plus your `package.json`, so BrowserStack support can investigate configuration issues without asking you to reproduce them. +- Set `disableAutoCaptureLogs: true` in the service options, or `BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS=true`, to turn this upload off entirely. From e9999dd77c3b2ade9d52e7b073c198c2a9d819de Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Tue, 11 Aug 2026 19:04:00 +0530 Subject: [PATCH 03/14] feat(SDK-7250): log the full auto-capture archive manifest The config-capture line names only the config files, so regression automation had no way to assert that package.json and the service log actually made it into the tarball -- it could only infer it. Emit the complete entry list at debug level right before the archive is written, which is the one place the whole manifest is known. Consumed by BStackAutomation's SDK-7250 coverage (common_helper.assert_wdio_auto_capture_archive_contains). Co-Authored-By: Claude Opus 5 (1M context) --- packages/browserstack-service/src/util.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index 778ecc5..ff0e82f 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -1633,6 +1633,11 @@ export async function uploadLogs(user: string | undefined, key: string | undefin failure = `archive_add_failed [${archiveAddFailures.length}]: ${archiveAddFailures.join('; ')}`.substring(0, 300) } + // Full archive manifest: the only place the complete entry list is visible, so + // regression automation can assert package.json and the config files actually + // made it in rather than inferring it from the config-capture line alone. + BStackLogger.debug(`Auto-capture archive entries: ${copiedFileNames.join(', ')}`) + await create( { file: tarPath, From 97e2f83f8ccda61c0397a7c55253cf1905e0dbcf Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Tue, 11 Aug 2026 21:16:04 +0530 Subject: [PATCH 04/14] fix(SDK-7250): address PR review - compound secret keys, package.json, dedup, path Review findings, all verified against a real uploaded bundle before and after. 1. Redaction missed compound secret keys. The whole-word pass rejects the letter before `Secret`/`Token`/`Key`, so `clientSecret` / `refreshToken` / `privateKey` survived it -- and so did snake_case `client_secret`, which the review did not mention. Added a second pass anchored on the SUFFIX. It is deliberately case-sensitive: requiring a capitalised suffix (camelCase) or an explicit `_` (snake_case) is what separates `privateKey` from `hotkey` and `client_secret` from `keyword`, so the leak closes without the false positives a bare /key|token|secret/ pass would produce. 2. package.json was the one capture path that skipped redaction. It now goes through redactSensitiveContent like the configs -- `scripts` routinely embed tokens (`--token=ghp_...`). Dependency and version lines are unaffected by the scrub. 3. The resolved config path was logged absolute into a log file that is itself uploaded, leaking the OS username. Now logged cwd-relative; `path.relative` still yields `../../shared/wdio.conf.ts` for a config outside cwd, so the monorepo diagnostic survives. 4. Basename de-duplication existed twice with different loop bounds. Extracted `dedupeEntryName` and used it in both places, which also removes the bounded-loop fallthrough in the configCapture copy that could have returned an already-taken name. The architecture comment (file I/O in the thin service layer) needs no change and is answered in-thread: the user's wdio.conf lives on the service host, not anywhere the binary can read, and this is co-located with the pre-existing log-upload I/O. Co-Authored-By: Claude Opus 5 (1M context) --- .../browserstack-service/src/configCapture.ts | 65 ++++++++++++++----- .../browserstack-service/src/constants.ts | 9 +++ packages/browserstack-service/src/util.ts | 42 ++++++------ .../tests/configCapture.test.ts | 53 +++++++++++++++ .../tests/uploadLogsArchive.test.ts | 19 ++++++ 5 files changed, 152 insertions(+), 36 deletions(-) diff --git a/packages/browserstack-service/src/configCapture.ts b/packages/browserstack-service/src/configCapture.ts index 87d9be7..0828344 100644 --- a/packages/browserstack-service/src/configCapture.ts +++ b/packages/browserstack-service/src/configCapture.ts @@ -6,6 +6,8 @@ import type { Options } from '@wdio/types' import { BStackLogger } from './bstackLogger.js' import { BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS, + COMPOUND_SECRET_SUFFIXES_CAMEL, + COMPOUND_SECRET_SUFFIXES_SNAKE, BROWSERSTACK_WDIO_CONFIG_FILE_PATH, BROWSERSTACK_WDIO_CONFIG_STRATEGY, CAPTURE_CONFIG_IMPORT_DEPTH, @@ -59,6 +61,15 @@ const resolveCandidate = (value: unknown): string | undefined => { } } +/** cwd-relative form of a path, for logs that get uploaded. Falls back to the input. */ +const relativeToCwd = (filePath: string): string => { + try { + return path.relative(process.cwd(), filePath) || path.basename(filePath) + } catch { + return path.basename(filePath) + } +} + const probeConfigBasename = (dir: string, basename: string): string | undefined => { for (const ext of SUPPORTED_WDIO_CONFIG_EXTENSIONS) { const candidate = path.join(dir, `${basename}${ext}`) @@ -220,9 +231,13 @@ export function initWdioConfigPath(config?: Options.Testrunner): ConfigPathResol if (resolution.strategy) { process.env[BROWSERSTACK_WDIO_CONFIG_STRATEGY] = resolution.strategy } - // Path first: BStackLogger scrubs any `<...>key:`/`<...>user:` prefixed value, so a - // strategy name ending in `key`/`user` right before the path would redact the path. - BStackLogger.debug(`Resolved wdio config file ${resolution.configPath} for auto-capture (strategy ${resolution.strategy})`) + // Relative to cwd: this log file is itself uploaded, and an absolute path leaks + // the OS username and directory layout (`/Users/jane.doe/...`). `path.relative` + // still yields `../../shared/wdio.conf.ts` for a config outside cwd, so the + // monorepo/subdir diagnostic — which is the whole point of logging it — survives. + // Path before strategy: BStackLogger scrubs any `<...>key:`/`<...>user:` prefixed + // value, so a strategy name ending in `key`/`user` here would redact the path. + BStackLogger.debug(`Resolved wdio config file ${relativeToCwd(resolution.configPath)} for auto-capture (strategy ${resolution.strategy})`) } else { BStackLogger.debug(`Could not resolve wdio config file for auto-capture: ${resolution.reason}`) } @@ -283,7 +298,25 @@ export function redactSensitiveContent(text: string): string { .join('|') const redactRegex = new RegExp(`^.*?(? { @@ -382,8 +415,11 @@ const collectLocalImports = (entryPath: string, entryContent: string, budget: nu * Give every archive entry a unique name. Two configs can share a basename * (`configs/wdio.conf.ts` + `shared/wdio.conf.ts`); without this the second silently * overwrites the first, since archive entries are keyed by basename. + * + * Shared with `uploadLogs`, which de-dupes the copied log files against these entries — + * one implementation so the two can never disagree. */ -const uniqueEntryName = (filePath: string, taken: Set): string => { +export function dedupeEntryName(filePath: string, taken: Set): string { const base = path.basename(filePath) if (!taken.has(base)) { taken.add(base) @@ -392,15 +428,14 @@ const uniqueEntryName = (filePath: string, taken: Set): string => { const ext = path.extname(base) const stem = ext ? base.slice(0, -ext.length) : base - for (let i = 1; i < MAX_CAPTURED_CONFIG_FILES + 2; i++) { - const candidate = `${stem}.${i}${ext}` - if (!taken.has(candidate)) { - taken.add(candidate) - return candidate - } + let index = 1 + let candidate = `${stem}.${index}${ext}` + while (taken.has(candidate)) { + index++ + candidate = `${stem}.${index}${ext}` } - taken.add(base) - return base + taken.add(candidate) + return candidate } /** @@ -429,7 +464,7 @@ export function collectConfigFilesForUpload(config?: Options.Testrunner): { file } files.push({ - name: uniqueEntryName(resolution.configPath, takenNames), + name: dedupeEntryName(resolution.configPath, takenNames), sourcePath: resolution.configPath, content: redactSensitiveContent(content) }) @@ -445,7 +480,7 @@ export function collectConfigFilesForUpload(config?: Options.Testrunner): { file continue } files.push({ - name: uniqueEntryName(importedPath, takenNames), + name: dedupeEntryName(importedPath, takenNames), sourcePath: importedPath, content: redactSensitiveContent(imported.content) }) diff --git a/packages/browserstack-service/src/constants.ts b/packages/browserstack-service/src/constants.ts index f489b4a..88fff57 100644 --- a/packages/browserstack-service/src/constants.ts +++ b/packages/browserstack-service/src/constants.ts @@ -81,6 +81,15 @@ export const MAX_PACKAGE_JSON_WALK_UP = 5 * Keys whose line is scrubbed before a config file enters the archive. * `user` / `key` are WDIO's own top-level credential options, hence the bare entries. */ +/** + * Word families that make an identifier sensitive when they appear as its SUFFIX — + * `clientSecret`, `refreshToken`, `privateKey`, `client_secret`. Split by case so the + * camelCase form requires a capital (distinguishing `privateKey` from `hotkey`) and the + * snake_case form requires an explicit `_` (distinguishing `client_secret` from `keyword`). + */ +export const COMPOUND_SECRET_SUFFIXES_CAMEL = 'Key|Token|Secret|Password|Passwd|Credential' +export const COMPOUND_SECRET_SUFFIXES_SNAKE = 'key|token|secret|password|passwd|credential' + export const REDACTED_KEYS = [ 'user', 'key', 'userName', 'accessKey', diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index ff0e82f..18c0c56 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -48,7 +48,7 @@ import { } from './constants.js' import CrashReporter from './crash-reporter.js' import { BStackLogger } from './bstackLogger.js' -import { collectConfigFilesForUpload, findPackageJsonForUpload, isAutoCaptureLogsDisabled } from './configCapture.js' +import { collectConfigFilesForUpload, dedupeEntryName, findPackageJsonForUpload, isAutoCaptureLogsDisabled, redactSensitiveContent } from './configCapture.js' import UsageStats from './testOps/usageStats.js' import TestOpsConfig from './testOps/testOpsConfig.js' import type { StartBinSessionResponse } from './grpc/index.js' @@ -1565,31 +1565,13 @@ export async function uploadLogs(user: string | undefined, key: string | undefin // Archive entries are keyed by basename, so two sources sharing one basename would // silently overwrite each other — reachable now that user-supplied config paths // (e.g. configs/wdio.conf.ts + shared/wdio.conf.ts) join the archive. + // Same helper collectConfigFilesForUpload uses, so the two cannot disagree. const takenNames = new Set(['logs.tar', 'logs.tar.gz']) - const uniqueName = (filePath: string): string => { - const base = path.basename(filePath) - if (!takenNames.has(base)) { - takenNames.add(base) - return base - } - const ext = path.extname(base) - const stem = ext ? base.slice(0, -ext.length) : base - let index = 1 - let candidate = `${stem}.${index}${ext}` - while (takenNames.has(candidate)) { - index++ - candidate = `${stem}.${index}${ext}` - } - takenNames.add(candidate) - return candidate - } + const uniqueName = (filePath: string): string => dedupeEntryName(filePath, takenNames) const filesToArchive = [ BStackLogger.logFilePath, CLI_DEBUG_LOGS_FILE, - // framework/service versions — first thing triage needs, and the archive - // carried neither before (the Node SDK has shipped package.json for years) - findPackageJsonForUpload(), ].filter((f): f is string => Boolean(f) && fs.existsSync(f as string)) const copiedFileNames: string[] = [] @@ -1609,6 +1591,24 @@ export async function uploadLogs(user: string | undefined, key: string | undefin // Soft-failure by design — a config we cannot read or locate must never stop the // log upload; the reason is recorded on the event instead. const { files: configFiles, failures: configFailures, strategy } = collectConfigFilesForUpload(options.config) + + // package.json gives triage the framework/service versions, which the archive carried + // before this change. It goes through the same redaction as the configs rather than + // being copied verbatim: `scripts` routinely embed tokens (`--token=ghp_...`), and the + // walk-up can select a monorepo-root manifest broader than the test project. Ordinary + // dependency/version lines are unaffected by the scrub. + const packageJsonPath = findPackageJsonForUpload() + if (packageJsonPath) { + try { + configFiles.push({ + name: path.basename(packageJsonPath), + sourcePath: packageJsonPath, + content: redactSensitiveContent(fs.readFileSync(packageJsonPath, 'utf8')) + }) + } catch (pkgErr) { + configFailures.push(`package.json: ${(pkgErr as Error)?.message || String(pkgErr)}`) + } + } for (const configFile of configFiles) { try { const entryName = uniqueName(configFile.name) diff --git a/packages/browserstack-service/tests/configCapture.test.ts b/packages/browserstack-service/tests/configCapture.test.ts index 1656e4e..da62866 100644 --- a/packages/browserstack-service/tests/configCapture.test.ts +++ b/packages/browserstack-service/tests/configCapture.test.ts @@ -223,6 +223,59 @@ describe('redactSensitiveContent', () => { .not.toContain('leaked-key') }) + it('scrubs compound camelCase secret keys (PR review, SDK-7250)', () => { + // The whole-word pass cannot see these: its lookbehind rejects the letter before + // `Secret`/`Token`/`Key`, so third-party secrets under compound names survived it. + const redacted = redactSensitiveContent([ + "clientSecret: 'cs_live_leak'", + "refreshToken: 'ya29_leak'", + "privateKey: '-----BEGIN PRIVATE KEY-----'", + "sessionSecret: 'sess_leak'", + 'bearerToken = "bt_leak"' + ].join('\n')) + + expect(redacted).not.toContain('cs_live_leak') + expect(redacted).not.toContain('ya29_leak') + expect(redacted).not.toContain('BEGIN PRIVATE KEY') + expect(redacted).not.toContain('sess_leak') + expect(redacted).not.toContain('bt_leak') + }) + + it('scrubs snake_case secret keys', () => { + const redacted = redactSensitiveContent([ + "client_secret: 'snake_leak'", + "refresh_token: 'snake_token_leak'", + "private_key: 'snake_key_leak'" + ].join('\n')) + + expect(redacted).not.toContain('snake_leak') + expect(redacted).not.toContain('snake_token_leak') + expect(redacted).not.toContain('snake_key_leak') + }) + + it('still does not over-redact lookalike identifiers', () => { + // Case sensitivity is what buys this: a bare /key|token|secret/ pass would take + // all of these with it. + const redacted = redactSensitiveContent([ + "hotkey: 'ctrl+a'", + "keyword: 'search'", + "monkeypatch: 'enabled'", + "tokenizer: 'default'", + "secretary: 'name'" + ].join('\n')) + + expect(redacted).toContain('ctrl+a') + expect(redacted).toContain('search') + expect(redacted).toContain('enabled') + expect(redacted).toContain('default') + expect(redacted).toContain('name') + }) + + it('scrubs a token embedded in a package.json script', () => { + expect(redactSensitiveContent('"deploy": "gh release upload --token=ghp_leak"')) + .not.toContain('ghp_leak') + }) + it('returns falsy input unchanged', () => { expect(redactSensitiveContent('')).toBe('') }) diff --git a/packages/browserstack-service/tests/uploadLogsArchive.test.ts b/packages/browserstack-service/tests/uploadLogsArchive.test.ts index f24c3f6..b081afe 100644 --- a/packages/browserstack-service/tests/uploadLogsArchive.test.ts +++ b/packages/browserstack-service/tests/uploadLogsArchive.test.ts @@ -86,6 +86,25 @@ describe('uploadLogs archive contents (SDK-7250)', () => { expect(entries).toContain('bstack-wdio-service.log') }) + it('redacts package.json instead of archiving it verbatim (PR review, SDK-7250)', async () => { + fs.writeFileSync(path.join(tmpProject, 'package.json'), JSON.stringify({ + name: 'customer-app', + version: '1.2.3', + scripts: { deploy: 'gh release upload --token=ghp_LEAKED_IN_MANIFEST' }, + dependencies: { webdriverio: '^9.0.0' } + }, null, 2)) + fs.writeFileSync(path.join(tmpProject, 'wdio.conf.js'), 'export const config = {}') + + await uploadLogs('some_user', 'some_key', 'some_uuid', {}) + + const raw = zlib.gunzipSync(uploadedArchive!).toString('binary') + expect(raw).not.toContain('ghp_LEAKED_IN_MANIFEST') + // the reason we ship it at all must survive the scrub + expect(raw).toContain('webdriverio') + expect(raw).toContain('1.2.3') + expect(await readArchiveEntries(uploadedArchive!)).toContain('package.json') + }) + it('scrubs credentials out of the archived config', async () => { fs.writeFileSync(path.join(tmpProject, 'wdio.conf.js'), [ 'export const config = {', From 894bd10f8f8f1a9788a5b17c6f8be10355f76c40 Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Tue, 11 Aug 2026 23:02:55 +0530 Subject: [PATCH 05/14] fix(SDK-7250): scrub SCREAMING_SNAKE keys, PEM blocks and basic-auth URLs Second review round. All three gaps reproduced against the head regex first, then verified closed on a real uploaded bundle. 1. SCREAMING_SNAKE_CASE bypassed the scrub entirely. The snake branch listed lowercase suffixes only and the compound pass carried no `i` flag, while the whole-word pass rejects any token preceded by `_`. So `CLIENT_SECRET`, `AWS_SECRET_ACCESS_KEY`, `GITHUB_TOKEN` and `REFRESH_TOKEN` were all untouched -- the dominant convention for secrets in config and env files. The snake branch is now matched case-insensitively, which is safe precisely because it requires an explicit `_` before the suffix: `HOTKEY`, `KEYWORD` and `my_secretary` still fall out. camelCase stays case-sensitive for the same reason as before. 2. A multi-line PEM leaked its key bytes. The line naming `privateKey` was scrubbed but the base64 body carries no key name, and every pass was line-anchored. Added a block-level pass that collapses `-----BEGIN ...-----` through `-----END ...-----` as a unit. 3. Basic-auth credentials leaked from any URL that was not `proxyUrl`. Added a userinfo rewrite so `https://admin:pass@host` becomes `https://[REDACTED]@host` for any scheme. A port-bearing URL with no userinfo (`https://example.com:8080/path`) is left alone. Block-level passes run before the line-anchored ones, since the latter can only ever see the single line that carries the key name. Also qualified the user-facing claim, which is the honest description now that the residual is known: the changeset and the `disableAutoCaptureLogs` doc say values under known credential keys are removed on a best-effort basis, and that a secret under an unrecognised name can still be included. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/pr-128.md | 2 +- .../browserstack-service/src/configCapture.ts | 38 +++++++----- .../browserstack-service/src/constants.ts | 9 +++ packages/browserstack-service/src/types.ts | 13 ++++- .../tests/configCapture.test.ts | 58 +++++++++++++++++++ 5 files changed, 103 insertions(+), 17 deletions(-) diff --git a/.changeset/pr-128.md b/.changeset/pr-128.md index 8499bbb..13773db 100644 --- a/.changeset/pr-128.md +++ b/.changeset/pr-128.md @@ -2,5 +2,5 @@ "@wdio/browserstack-service": minor --- -- The debug logs the service uploads at the end of a run now include a copy of your `wdio.conf` file (and the local config files it imports) with credentials removed, plus your `package.json`, so BrowserStack support can investigate configuration issues without asking you to reproduce them. +- The debug logs the service uploads at the end of a run now include a copy of your `wdio.conf` file (and the local config files it imports) plus your `package.json`, with values under known credential keys removed on a best-effort basis, so BrowserStack support can investigate configuration issues without asking you to reproduce them. - Set `disableAutoCaptureLogs: true` in the service options, or `BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS=true`, to turn this upload off entirely. diff --git a/packages/browserstack-service/src/configCapture.ts b/packages/browserstack-service/src/configCapture.ts index 0828344..6319869 100644 --- a/packages/browserstack-service/src/configCapture.ts +++ b/packages/browserstack-service/src/configCapture.ts @@ -8,6 +8,8 @@ import { BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS, COMPOUND_SECRET_SUFFIXES_CAMEL, COMPOUND_SECRET_SUFFIXES_SNAKE, + PEM_BLOCK_REGEX, + URL_USERINFO_REGEX, BROWSERSTACK_WDIO_CONFIG_FILE_PATH, BROWSERSTACK_WDIO_CONFIG_STRATEGY, CAPTURE_CONFIG_IMPORT_DEPTH, @@ -298,25 +300,35 @@ export function redactSensitiveContent(text: string): string { .join('|') const redactRegex = new RegExp(`^.*?(?` + // immediately before an assignment. + const compoundSnakeRegex = new RegExp( + `^.*?(? { diff --git a/packages/browserstack-service/src/constants.ts b/packages/browserstack-service/src/constants.ts index 88fff57..ddfbb69 100644 --- a/packages/browserstack-service/src/constants.ts +++ b/packages/browserstack-service/src/constants.ts @@ -90,6 +90,15 @@ export const MAX_PACKAGE_JSON_WALK_UP = 5 export const COMPOUND_SECRET_SUFFIXES_CAMEL = 'Key|Token|Secret|Password|Passwd|Credential' export const COMPOUND_SECRET_SUFFIXES_SNAKE = 'key|token|secret|password|passwd|credential' +/** + * Secrets that span lines or hide inside a value, which a line/key-anchored scrub cannot + * reach. Applied as whole-block passes before the line passes. + */ +/* an inline PEM: the key bytes sit on lines that carry no key name at all */ +export const PEM_BLOCK_REGEX = /(-----BEGIN [^-\r\n]+-----)[\s\S]*?(-----END [^-\r\n]+-----)/g +/* basic-auth userinfo in ANY url value, not just the `proxyUrl` key */ +export const URL_USERINFO_REGEX = /([a-zA-Z][a-zA-Z0-9+.-]*:\/\/)[^\s/@:]+:[^\s/@]*@/g + export const REDACTED_KEYS = [ 'user', 'key', 'userName', 'accessKey', diff --git a/packages/browserstack-service/src/types.ts b/packages/browserstack-service/src/types.ts index b4e50ae..d8d8fab 100644 --- a/packages/browserstack-service/src/types.ts +++ b/packages/browserstack-service/src/types.ts @@ -104,9 +104,16 @@ export interface BrowserstackConfig { */ testManagementOptions?: TestManagementOptions; /** - * By default the service uploads its own debug logs, your `package.json` and a - * credential-redacted copy of your wdio config file at the end of a run, so - * BrowserStack support can debug issues without asking you to reproduce them. + * By default the service uploads its own debug logs, your `package.json` and a copy of + * your wdio config file at the end of a run, so BrowserStack support can debug issues + * without asking you to reproduce them. + * + * Values under known credential keys are removed before upload on a best-effort basis: + * BrowserStack credentials, common third-party secret names (`clientSecret`, + * `AWS_SECRET_ACCESS_KEY`, …), inline PEM blocks and basic-auth URLs. It is key-name + * driven, so a secret stored under an unrecognised name can still be included — if your + * config holds secrets you would rather not send, set this to true. + * * Set this to true to disable that upload entirely. * Can also be set with the `BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS=true` env var. * @default false diff --git a/packages/browserstack-service/tests/configCapture.test.ts b/packages/browserstack-service/tests/configCapture.test.ts index da62866..78f4913 100644 --- a/packages/browserstack-service/tests/configCapture.test.ts +++ b/packages/browserstack-service/tests/configCapture.test.ts @@ -271,6 +271,64 @@ describe('redactSensitiveContent', () => { expect(redacted).toContain('name') }) + it('scrubs SCREAMING_SNAKE_CASE secret keys (PR review round 2)', () => { + // The dominant convention for secrets in config/env files. The snake branch is + // matched case-insensitively, which is safe because it requires an explicit `_`. + const redacted = redactSensitiveContent([ + "CLIENT_SECRET: 'screaming_leak'", + "AWS_SECRET_ACCESS_KEY: 'AKIA_leak'", + "GITHUB_TOKEN = 'ghp_screaming_leak'", + "REFRESH_TOKEN: 'rt_screaming_leak'", + "export const DB_PASSWORD = 'pw_screaming_leak'" + ].join('\n')) + + expect(redacted).not.toContain('screaming_leak') + expect(redacted).not.toContain('AKIA_leak') + expect(redacted).not.toContain('ghp_screaming_leak') + expect(redacted).not.toContain('rt_screaming_leak') + expect(redacted).not.toContain('pw_screaming_leak') + }) + + it('does not over-redact uppercase lookalikes', () => { + const redacted = redactSensitiveContent([ + "HOTKEY: 'ctrl+a'", + "KEYWORD: 'search'", + "my_secretary: 'jane'" + ].join('\n')) + + expect(redacted).toContain('ctrl+a') + expect(redacted).toContain('search') + expect(redacted).toContain('jane') + }) + + it('scrubs a multi-line PEM block, not just the line naming it', () => { + const redacted = redactSensitiveContent([ + 'credentials: {', + ' privateKey: `-----BEGIN PRIVATE KEY-----', + 'MIIEvQIBADANBgkqhkiG9w0BAQEFAASCsecretbytes', + '-----END PRIVATE KEY-----`', + '}' + ].join('\n')) + + expect(redacted).not.toContain('MIIEvQIBADANBgkqhkiG9w0BAQEFAASCsecretbytes') + }) + + it('scrubs basic-auth credentials embedded in any URL, not just proxyUrl', () => { + const redacted = redactSensitiveContent([ + "baseUrl: 'https://admin:s3cr3tPass@example.com'", + "mongoUri: 'mongodb://dbuser:dbP4ss@cluster0.example.net'" + ].join('\n')) + + expect(redacted).not.toContain('s3cr3tPass') + expect(redacted).not.toContain('dbP4ss') + expect(redacted).toContain('example.com') + }) + + it('leaves a port-bearing URL with no userinfo alone', () => { + expect(redactSensitiveContent("baseUrl: 'https://example.com:8080/path'")) + .toContain('https://example.com:8080/path') + }) + it('scrubs a token embedded in a package.json script', () => { expect(redactSensitiveContent('"deploy": "gh release upload --token=ghp_leak"')) .not.toContain('ghp_leak') From e5930937db275b02223878c49a2ae1dae695e205 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:33:10 +0000 Subject: [PATCH 06/14] chore(changeset): auto-generate from PR template (minor) --- .changeset/pr-128.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/pr-128.md b/.changeset/pr-128.md index 13773db..8499bbb 100644 --- a/.changeset/pr-128.md +++ b/.changeset/pr-128.md @@ -2,5 +2,5 @@ "@wdio/browserstack-service": minor --- -- The debug logs the service uploads at the end of a run now include a copy of your `wdio.conf` file (and the local config files it imports) plus your `package.json`, with values under known credential keys removed on a best-effort basis, so BrowserStack support can investigate configuration issues without asking you to reproduce them. +- The debug logs the service uploads at the end of a run now include a copy of your `wdio.conf` file (and the local config files it imports) with credentials removed, plus your `package.json`, so BrowserStack support can investigate configuration issues without asking you to reproduce them. - Set `disableAutoCaptureLogs: true` in the service options, or `BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS=true`, to turn this upload off entirely. From 45d25d73378040e60472be0caba3a45f92b16eec Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Tue, 11 Aug 2026 23:37:55 +0530 Subject: [PATCH 07/14] fix(SDK-7250): bound the URL userinfo scan, catch single-token userinfo and open PEMs Third review round. 1. ReDoS. Measured: URL_USERINFO_REGEX is quadratic -- 12.5k chars 97ms, 25k 382ms, 50k 1543ms, 100k 6144ms, 4x per doubling, because both userinfo halves scan forward for an `@` that never arrives. redactSensitiveContent runs synchronously inside uploadLogs, so a captured config carrying one long unbroken run (a base64/data: URI, a minified line) would block the event loop for minutes and stall exit. Bounded the quantifiers: 100k drops 6144ms -> 20ms, 400k -> 83ms, linear. Added a linearity guard test. The same report also called the two compound identifier passes quadratic. That did NOT reproduce: 0-1ms at every size I could construct, including the suffix literal present with no assignment, many suffix occurrences on one line, a 105k base64 data: URI, and the report's own stated input (100k word chars + trailing colon) at 1ms rather than 6155ms. The required literal suffix bounds the backtracking. Bounded them at 64 chars anyway -- real config keys are far shorter, so it costs nothing and hardens a case I could not build. 2. Single-token URL userinfo leaked: the pattern required `user:pass@`, so `https://ghp_xxx@github.com` -- the shape CI git remotes and npm registry auth use -- was untouched. Password half is now optional. 3. An unterminated PEM (BEGIN with no END) leaked its body, since the block pass needs the END marker and the body lines carry no key name. Added a bounded pass matching BEGIN plus the run of base64-only lines that follows. Two bugs in my own round-3 fixes, both caught by testing rather than review: - The first cut of the unterminated-PEM pass ate ordinary lines. Letters are valid base64, so it matched `nextOption` out of `nextOption: 1`. The body run must now be at least 20 characters AND end at a non-base64 character. - Live-bundle verification then showed PEM_BLOCK_REGEX spanning from an unterminated BEGIN through to a LATER, unrelated block's END marker, replacing every line in between and silently destroying unrelated config. The body is now tempered so it cannot cross a second BEGIN, and bounded so the scan stays linear. Verified on a real uploaded bundle: all ten planted leak vectors absent from the archived config, all six triage markers still readable. Co-Authored-By: Claude Opus 5 (1M context) --- .../browserstack-service/src/configCapture.ts | 9 ++- .../browserstack-service/src/constants.ts | 27 ++++++++- .../tests/configCapture.test.ts | 58 +++++++++++++++++++ 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/packages/browserstack-service/src/configCapture.ts b/packages/browserstack-service/src/configCapture.ts index 6319869..4b8757f 100644 --- a/packages/browserstack-service/src/configCapture.ts +++ b/packages/browserstack-service/src/configCapture.ts @@ -9,6 +9,7 @@ import { COMPOUND_SECRET_SUFFIXES_CAMEL, COMPOUND_SECRET_SUFFIXES_SNAKE, PEM_BLOCK_REGEX, + PEM_UNTERMINATED_REGEX, URL_USERINFO_REGEX, BROWSERSTACK_WDIO_CONFIG_FILE_PATH, BROWSERSTACK_WDIO_CONFIG_STRATEGY, @@ -307,8 +308,11 @@ export function redactSensitiveContent(text: string): string { // camelCase stays case-SENSITIVE: requiring a capitalised suffix is what separates // `privateKey` from `hotkey`, so this closes the leak without the false positives a // bare /key|token|secret/ pass would produce. + // Identifier scans are bounded at 64 chars. Real config keys are far shorter, so this + // changes no match; it is defence-in-depth against backtracking on a pathological line + // (a minified/base64 run), keeping every start position O(1) instead of O(n). const compoundCamelRegex = new RegExp( - `^.*?(?` // immediately before an assignment. const compoundSnakeRegex = new RegExp( - `^.*?(? { .toContain('https://example.com:8080/path') }) + it('scrubs single-token userinfo in a URL, not just user:pass (PR review round 3)', () => { + // Common in CI git remotes and npm registry auth. + const redacted = redactSensitiveContent([ + "repoUrl: 'https://ghp_TOKENLEAK@github.com/x/y.git'", + "registry: 'https://npm_TOKENLEAK2@registry.example.com'" + ].join('\n')) + + expect(redacted).not.toContain('ghp_TOKENLEAK') + expect(redacted).not.toContain('npm_TOKENLEAK2') + expect(redacted).toContain('github.com') + }) + + it('scrubs an unterminated PEM block without eating the rest of the file', () => { + const redacted = redactSensitiveContent([ + 'credentials: {', + ' privateKey: `-----BEGIN PRIVATE KEY-----', + 'MIIEvQIBADANunterminatedbytes', + 'nextOption: 1', + '}' + ].join('\n')) + + expect(redacted).not.toContain('MIIEvQIBADANunterminatedbytes') + // a malformed block must not swallow everything after it + expect(redacted).toContain('nextOption') + }) + + it('does not let an unterminated PEM swallow a later, unrelated PEM block', () => { + // Found live: with an untempered body the FIRST (unterminated) BEGIN matched through + // to the SECOND block's END marker, replacing every unrelated line in between. + const redacted = redactSensitiveContent([ + 'openPem: `-----BEGIN RSA PRIVATE KEY-----', + 'MIIEUNTERMINATEDBYTESMUSTNOTAPPEAR`,', + "afterPem: 'STILL_READABLE_MARKER',", + 'pem: `-----BEGIN PRIVATE KEY-----', + 'MIIEvQTERMINATEDBYTESMUSTNOTAPPEAR', + '-----END PRIVATE KEY-----`,' + ].join('\n')) + + expect(redacted).not.toContain('MIIEUNTERMINATEDBYTESMUSTNOTAPPEAR') + expect(redacted).not.toContain('MIIEvQTERMINATEDBYTESMUSTNOTAPPEAR') + // the line between the two blocks must survive + expect(redacted).toContain('STILL_READABLE_MARKER') + }) + + it('stays linear on pathological input (ReDoS guard)', () => { + // The unbounded URL userinfo pass was measurably quadratic: 100 KB of word + // characters took ~6.1s, 4x per doubling, and redactSensitiveContent runs + // synchronously inside uploadLogs. Bounded quantifiers keep it flat. + const build = (n: number) => `baseUrl: 'https://${'a'.repeat(n)}` + '\n' + `k${'b'.repeat(n)}Key` + + const started = Date.now() + redactSensitiveContent(build(200_000)) + const elapsed = Date.now() - started + + // generous ceiling: the unbounded form did not finish 1 MB in 120s + expect(elapsed).toBeLessThan(2_000) + }) + it('scrubs a token embedded in a package.json script', () => { expect(redactSensitiveContent('"deploy": "gh release upload --token=ghp_leak"')) .not.toContain('ghp_leak') From e5b649e6a468eccd258e296831f719a46ec41448 Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Tue, 11 Aug 2026 23:38:46 +0530 Subject: [PATCH 08/14] docs(SDK-7250): keep the best-effort redaction wording in the changeset The changeset bot regenerates .changeset/pr-128.md from the PR body, which still carried the unqualified 'with credentials removed'. Updated the PR body release note as well so the two agree and the qualification survives the next regeneration. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/pr-128.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/pr-128.md b/.changeset/pr-128.md index 8499bbb..13773db 100644 --- a/.changeset/pr-128.md +++ b/.changeset/pr-128.md @@ -2,5 +2,5 @@ "@wdio/browserstack-service": minor --- -- The debug logs the service uploads at the end of a run now include a copy of your `wdio.conf` file (and the local config files it imports) with credentials removed, plus your `package.json`, so BrowserStack support can investigate configuration issues without asking you to reproduce them. +- The debug logs the service uploads at the end of a run now include a copy of your `wdio.conf` file (and the local config files it imports) plus your `package.json`, with values under known credential keys removed on a best-effort basis, so BrowserStack support can investigate configuration issues without asking you to reproduce them. - Set `disableAutoCaptureLogs: true` in the service options, or `BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS=true`, to turn this upload off entirely. From 5ebc3182d96b31f817598d6d30898dd83d51c1bf Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Thu, 13 Aug 2026 18:38:06 +0530 Subject: [PATCH 09/14] fix(SDK-7250): manifest into the archive, acronym secret keys, disclosure notice Fourth review round. Five findings, all reproduced before fixing. 1. The capture manifest never reached the archive. The service log is snapshotted into the staging dir before the manifest and capture lines are written, so everything logged after that copy stayed only in the local file. Confirmed on a controlled single run: the archived log was 2 178 bytes shorter than the local one and contained neither line. Support downloading a bundle saw no manifest, no resolution strategy and no capture failures. The manifest is now its own archive entry (capture-manifest.txt) carrying the entry list, the strategy, the captured config names, the package.json location and any failures -- inside the tarball by construction rather than by ordering luck. Worth noting this also invalidated a regression test: the BStackAutomation coverage asserts the manifest by reading the LOCAL log, so it passed while the archive lacked it. 2. Acronym-prefixed camelCase keys leaked: APIToken, JWTSecret, SSHKey, AWSSecret, OTPKey. The camel core required a lowercase char immediately before the suffix, so an uppercase acronym failed it, and the whole-word pass rejected `Token` for the same preceding `I`. That guard was never load-bearing -- the alternation is already case-sensitive, so `hotkey` can never match it regardless of what precedes. Relaxed the core; the full over-redaction corpus (hotkey, HOTKEY, keyword, monkeypatch, tokenizer, secretary, donkey) still survives. 3. No runtime disclosure. The Node SDK prints AUTOLOGCAPTURE_NOTIFICATION when auto-capture is active; a wdio customer got only a changeset entry and a JSDoc comment for a strictly broader capture. Added the equivalent info line naming what is collected and how to disable. 4. "Auto-captured 1 config file(s) via undefined: package.json" -- package.json was appended to the config list before the count was taken, and strategy is undefined on every failure path, so the line claimed success in exactly the case where no config was captured. Config files and package.json are now logged separately, and the strategy clause is dropped when absent. 5. Imported configs were read from disk twice: once to seed the import frontier, discarded, then again to archive. Beyond the wasted read the two could disagree if a file changed in between. collectLocalImports now returns the content it already has. Also raised the unterminated-PEM per-line bound from 200 to 8192 chars. The bounded quantifiers fail OPEN, so a key written unwrapped on a single line exceeded the bound, matched nothing and shipped. The URL userinfo bound is deliberately left as-is -- see the review reply. Co-Authored-By: Claude Opus 5 (1M context) --- .../browserstack-service/src/configCapture.ts | 27 ++++++-------- .../browserstack-service/src/constants.ts | 15 +++++++- packages/browserstack-service/src/launcher.ts | 2 + packages/browserstack-service/src/util.ts | 37 ++++++++++++++++--- .../tests/configCapture.test.ts | 37 +++++++++++++++++++ .../tests/uploadLogsArchive.test.ts | 25 +++++++++++++ 6 files changed, 121 insertions(+), 22 deletions(-) diff --git a/packages/browserstack-service/src/configCapture.ts b/packages/browserstack-service/src/configCapture.ts index 4b8757f..0c2f1df 100644 --- a/packages/browserstack-service/src/configCapture.ts +++ b/packages/browserstack-service/src/configCapture.ts @@ -307,12 +307,14 @@ export function redactSensitiveContent(text: string): string { // // camelCase stays case-SENSITIVE: requiring a capitalised suffix is what separates // `privateKey` from `hotkey`, so this closes the leak without the false positives a - // bare /key|token|secret/ pass would produce. + // bare /key|token|secret/ pass would produce. The core does NOT require a lowercase + // char before the suffix — that guard only ever duplicated the case-sensitivity, while + // rejecting acronym prefixes (`APIToken`, `JWTSecret`, `SSHKey`, `AWSSecret`). // Identifier scans are bounded at 64 chars. Real config keys are far shorter, so this // changes no match; it is defence-in-depth against backtracking on a pathological line // (a minified/base64 run), keeping every start position O(1) instead of O(n). const compoundCamelRegex = new RegExp( - `^.*?(? { - const found: string[] = [] +const collectLocalImports = (entryPath: string, entryContent: string, budget: number): Array<{ filePath: string, content: string }> => { + const found: Array<{ filePath: string, content: string }> = [] const seen = new Set([entryPath]) let frontier: Array<{ filePath: string, content: string }> = [{ filePath: entryPath, content: entryContent }] @@ -414,7 +416,7 @@ const collectLocalImports = (entryPath: string, entryContent: string, budget: nu if (importedContent === undefined) { continue } - found.push(resolved) + found.push({ filePath: resolved, content: importedContent }) next.push({ filePath: resolved, content: importedContent }) } } @@ -488,17 +490,12 @@ export function collectConfigFilesForUpload(config?: Options.Testrunner): { file const remaining = MAX_CAPTURED_CONFIG_FILES - files.length if (remaining > 0) { - for (const importedPath of collectLocalImports(resolution.configPath, content, remaining)) { - const imported = readCappedFile(importedPath) - if (imported.content === undefined) { - if (imported.reason) { - failures.push(imported.reason) - } - continue - } + // content comes back from the discovery pass — re-reading here would be a second + // disk read per file and could archive different bytes than were scanned. + for (const imported of collectLocalImports(resolution.configPath, content, remaining)) { files.push({ - name: dedupeEntryName(importedPath, takenNames), - sourcePath: importedPath, + name: dedupeEntryName(imported.filePath, takenNames), + sourcePath: imported.filePath, content: redactSensitiveContent(imported.content) }) } diff --git a/packages/browserstack-service/src/constants.ts b/packages/browserstack-service/src/constants.ts index a09289c..00dc2dd 100644 --- a/packages/browserstack-service/src/constants.ts +++ b/packages/browserstack-service/src/constants.ts @@ -56,6 +56,15 @@ export const PERCY_LOGS_FILE = 'logs/percy.log' * Auto-capture of the user's wdio config file (SDK-7250). */ +/* + * Shown once per run when auto-capture is active. The Node SDK does the same + * (BrowserStackSetup.js -> AUTOLOGCAPTURE_NOTIFICATION); without it a wdio customer gets no + * runtime disclosure that their config file is collected, only a changeset entry and a JSDoc + * comment. Since the redaction is key-name driven and best-effort, the notice is part of the + * control rather than decoration. + */ +export const AUTOLOGCAPTURE_NOTIFICATION = 'Your wdio config file, the local config files it imports and package.json are captured with the debug logs at the end of the run, with values under known credential keys removed. To disable, set disableAutoCaptureLogs: true in the browserstack service options.' + /* Absolute path of the resolved wdio config, published once so the upload path never re-derives it */ export const BROWSERSTACK_WDIO_CONFIG_FILE_PATH = 'BROWSERSTACK_WDIO_CONFIG_FILE_PATH' export const BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS = 'BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS' @@ -108,9 +117,11 @@ export const PEM_BLOCK_REGEX = /(-----BEGIN [^-\r\n]+-----)(?:(?!-----BEGIN)[\s\ * follows. A run must be at least 20 characters and end at a non-base64 character: letters * are valid base64, so a shorter/unbounded rule matches part of an ordinary line such as * `nextOption: 1` and eats it. Stops at the first line that does not qualify, so a malformed - * block cannot swallow the rest of the file. + * block cannot swallow the rest of the file. The upper bound is generous (8 KB) because a + * key written unwrapped on ONE line would otherwise exceed it and fail OPEN, leaving the body + * in the bundle. */ -export const PEM_UNTERMINATED_REGEX = /(-----BEGIN [^-\r\n]+-----)(?:\r?\n[A-Za-z0-9+/=]{20,200}(?=[^A-Za-z0-9+/=]|$))+/g +export const PEM_UNTERMINATED_REGEX = /(-----BEGIN [^-\r\n]+-----)(?:\r?\n[A-Za-z0-9+/=]{20,8192}(?=[^A-Za-z0-9+/=]|$))+/g /* * Userinfo in ANY url value, not just the `proxyUrl` key. The password half is optional so * single-token forms (`https://ghp_xxx@github.com`, common in CI git/npm remotes) are caught diff --git a/packages/browserstack-service/src/launcher.ts b/packages/browserstack-service/src/launcher.ts index a6eecfa..390e4e8 100644 --- a/packages/browserstack-service/src/launcher.ts +++ b/packages/browserstack-service/src/launcher.ts @@ -19,6 +19,7 @@ import type { BrowserstackConfig, BrowserstackOptions, App, AppConfig, AppUpload import { BSTACK_SERVICE_VERSION, NOT_ALLOWED_KEYS_IN_CAPS, PERF_MEASUREMENT_ENV, RERUN_ENV, RERUN_TESTS_ENV, + AUTOLOGCAPTURE_NOTIFICATION, BROWSERSTACK_TESTHUB_UUID, VALID_APP_EXTENSION, BROWSERSTACK_PERCY, @@ -257,6 +258,7 @@ export default class BrowserstackLauncherService implements Services.ServiceInst // SDK-5993 fixed in the Node SDK (silently dropped the config on every monorepo / // subdir CI run). Best-effort: never blocks the run. if (!publishAutoCaptureDisabled(this._options)) { + BStackLogger.info(AUTOLOGCAPTURE_NOTIFICATION) initWdioConfigPath(config) } diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index 18c0c56..a63df0e 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -1597,6 +1597,9 @@ export async function uploadLogs(user: string | undefined, key: string | undefin // being copied verbatim: `scripts` routinely embed tokens (`--token=ghp_...`), and the // walk-up can select a monorepo-root manifest broader than the test project. Ordinary // dependency/version lines are unaffected by the scrub. + // snapshot before package.json joins the list, so counts and names stay honest + const capturedConfigNames = configFiles.map(f => f.name) + const packageJsonPath = findPackageJsonForUpload() if (packageJsonPath) { try { @@ -1619,8 +1622,17 @@ export async function uploadLogs(user: string | undefined, key: string | undefin configFailures.push(`${configFile.name}: ${msg}`) } } - if (configFiles.length > 0) { - BStackLogger.debug(`Auto-captured ${configFiles.length} config file(s) via ${strategy}: ${configFiles.map(f => f.name).join(', ')}`) + // Logged separately from package.json: `configFiles` has the manifest appended to it, + // and `strategy` is undefined on every failure path, so a combined line reads + // "Auto-captured 1 config file(s) via undefined: package.json" — i.e. it claims a + // capture succeeded in exactly the case where none did. + if (capturedConfigNames.length > 0) { + BStackLogger.debug(`Auto-captured ${capturedConfigNames.length} config file(s) via ${strategy}: ${capturedConfigNames.join(', ')}`) + } else { + BStackLogger.debug(`No wdio config captured${strategy ? ` (strategy ${strategy})` : ''}`) + } + if (packageJsonPath) { + BStackLogger.debug(`Auto-captured package.json from ${path.dirname(packageJsonPath)}`) } if (configFailures.length > 0 && failure === undefined) { // Warning only — `success` stays true so a missing config never reads as a @@ -1633,9 +1645,24 @@ export async function uploadLogs(user: string | undefined, key: string | undefin failure = `archive_add_failed [${archiveAddFailures.length}]: ${archiveAddFailures.join('; ')}`.substring(0, 300) } - // Full archive manifest: the only place the complete entry list is visible, so - // regression automation can assert package.json and the config files actually - // made it in rather than inferring it from the config-capture line alone. + // Written as its OWN archive entry rather than only to the service log. The log file + // is snapshotted into the staging dir above, so anything logged after that copy never + // reaches the tarball — support downloading the bundle would see no manifest, no + // resolution strategy and no capture failures. Those are exactly where triage starts. + try { + const manifestName = uniqueName('capture-manifest.txt') + const manifestLines = [ + `archive entries: ${[...copiedFileNames, manifestName].join(', ')}`, + `config resolution strategy: ${strategy || 'none'}`, + `config files captured: ${capturedConfigNames.join(', ') || 'none'}`, + `package.json: ${packageJsonPath ? path.dirname(packageJsonPath) : 'not found'}`, + `capture failures: ${configFailures.join('; ') || 'none'}` + ] + fs.writeFileSync(path.join(tmpDir, manifestName), manifestLines.join('\n') + '\n') + copiedFileNames.push(manifestName) + } catch (manifestErr) { + BStackLogger.debug(`Failed to write capture manifest: ${getErrorString(manifestErr)}`) + } BStackLogger.debug(`Auto-capture archive entries: ${copiedFileNames.join(', ')}`) await create( diff --git a/packages/browserstack-service/tests/configCapture.test.ts b/packages/browserstack-service/tests/configCapture.test.ts index b0cd1fe..2817bca 100644 --- a/packages/browserstack-service/tests/configCapture.test.ts +++ b/packages/browserstack-service/tests/configCapture.test.ts @@ -387,6 +387,43 @@ describe('redactSensitiveContent', () => { expect(elapsed).toBeLessThan(2_000) }) + it('scrubs acronym-prefixed camelCase secret keys (PR review round 4)', () => { + // The old core required a lowercase char before the suffix, so an uppercase acronym + // prefix slipped past BOTH passes: the camel core rejected `APIToken` on the `I`, and + // the whole-word pass rejected `Token` for the same preceding `I`. + const redacted = redactSensitiveContent([ + "APIToken: 'API_LEAK'", + "JWTSecret: 'JWT_LEAK'", + "SSHKey: 'SSH_LEAK'", + "AWSSecret: 'AWS_LEAK'", + "OTPKey = 'OTP_LEAK'" + ].join('\n')) + + for (const leak of ['API_LEAK', 'JWT_LEAK', 'SSH_LEAK', 'AWS_LEAK', 'OTP_LEAK']) { + expect(redacted).not.toContain(leak) + } + }) + + it('still keeps lookalikes after relaxing the camel core', () => { + const redacted = redactSensitiveContent([ + "hotkey: 'ctrl+a'", "HOTKEY: 'ctrl+b'", "keyword: 'search'", + 'monkeypatch: 1', "tokenizer: 'x'", "secretary: 'jane'", "donkey: 'y'" + ].join('\n')) + + for (const keep of ['ctrl+a', 'ctrl+b', 'search', 'monkeypatch', 'tokenizer', 'secretary', 'donkey']) { + expect(redacted).toContain(keep) + } + }) + + it('scrubs an unterminated PEM whose body is one long unwrapped line', () => { + // The 200-char per-line bound made this fail OPEN: a key written unwrapped on a + // single line exceeded it, so nothing matched and the body shipped. + const body = 'B'.repeat(3000) + const redacted = redactSensitiveContent(`-----BEGIN PRIVATE KEY-----\n${body}`) + + expect(redacted).not.toContain(body) + }) + it('scrubs a token embedded in a package.json script', () => { expect(redactSensitiveContent('"deploy": "gh release upload --token=ghp_leak"')) .not.toContain('ghp_leak') diff --git a/packages/browserstack-service/tests/uploadLogsArchive.test.ts b/packages/browserstack-service/tests/uploadLogsArchive.test.ts index b081afe..43375aa 100644 --- a/packages/browserstack-service/tests/uploadLogsArchive.test.ts +++ b/packages/browserstack-service/tests/uploadLogsArchive.test.ts @@ -123,6 +123,31 @@ describe('uploadLogs archive contents (SDK-7250)', () => { expect(raw).toContain('accessibility: true') }) + it('puts the capture manifest INSIDE the archive, not just in the log', async () => { + // The service log is snapshotted into the staging dir before these lines are written, + // so anything logged afterwards never reaches the tarball. Support downloading the + // bundle needs the manifest, the strategy and the failures to be in it. + fs.writeFileSync(path.join(tmpProject, 'package.json'), '{"name":"app"}') + fs.writeFileSync(path.join(tmpProject, 'wdio.conf.js'), 'export const config = {}') + + await uploadLogs('some_user', 'some_key', 'some_uuid', {}) + + const entries = await readArchiveEntries(uploadedArchive!) + expect(entries).toContain('capture-manifest.txt') + + const raw = zlib.gunzipSync(uploadedArchive!).toString('binary') + expect(raw).toContain('config resolution strategy:') + expect(raw).toContain('wdio.conf.js') + }) + + it('records the reason in the manifest when no config is found', async () => { + await uploadLogs('some_user', 'some_key', 'some_uuid', {}) + + const raw = zlib.gunzipSync(uploadedArchive!).toString('binary') + expect(raw).toContain('capture failures:') + expect(raw).toContain('config files captured: none') + }) + it('still uploads the logs when no config can be found', async () => { await uploadLogs('some_user', 'some_key', 'some_uuid', {}) From 89a8c429bf8c87312a867d98a1b040e1546ecab7 Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Thu, 13 Aug 2026 19:23:33 +0530 Subject: [PATCH 10/14] =?UTF-8?q?fix(SDK-7250):=20drop=20the=20config.=5F?= =?UTF-8?q?=20resolution=20rung=20=E2=80=94=20dead=20and=20actively=20harm?= =?UTF-8?q?ful?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It looked like a free extra signal, but `config._` is derived from the same argv the scan above already reads, minus the scan's guard that skips a flag's value. So it can only ever differ by taking something the scan correctly rejected. Proven: with `wdio --spec ./a.js` and no config positional, that rung resolves the SPEC file as the config; without it the resolver correctly falls through to wdio.conf.js. Regression test added. Co-Authored-By: Claude Opus 5 (1M context) --- .../browserstack-service/src/configCapture.ts | 21 ++++++++----------- .../tests/configCapture.test.ts | 11 ++++++++++ 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/packages/browserstack-service/src/configCapture.ts b/packages/browserstack-service/src/configCapture.ts index 0c2f1df..7b103af 100644 --- a/packages/browserstack-service/src/configCapture.ts +++ b/packages/browserstack-service/src/configCapture.ts @@ -151,10 +151,15 @@ const scanArgvForConfig = (argv: string[]): string | undefined => { * positional, which survives into the merged * config object (v8 and v9 alike) * 3. process.argv positional — `wdio ` without the `run` subcommand - * 4. config._[0] — same positional as seen by yargs - * 5. rootDir + wdio.conf. — no-arg `wdio`, and programmatic `new Launcher()` - * 6. cwd + wdio.conf. — when the user overrides `rootDir` in their config - * 7. single `*.conf.` in rootDir/cwd — unambiguous custom filenames only + * 4. rootDir + wdio.conf. — no-arg `wdio`, and programmatic `new Launcher()` + * 5. cwd + wdio.conf. — when the user overrides `rootDir` in their config + * 6. single `*.conf.` in rootDir/cwd — unambiguous custom filenames only + * + * There is deliberately NO `config._` rung. It looks like a free extra signal, but `config._` + * is derived from the same argv the scan above already reads — without the scan's guard that + * skips a flag's value. So it only ever differs by taking something the scan correctly + * rejected: `wdio --spec ./a.js` resolves to the SPEC file under that rung, and to the real + * `wdio.conf.js` without it. */ export function resolveWdioConfigPath(config?: Options.Testrunner): ConfigPathResolution { const configRecord = (config || {}) as Record @@ -174,14 +179,6 @@ export function resolveWdioConfigPath(config?: Options.Testrunner): ConfigPathRe return { configPath: fromArgv, strategy: 'argv_positional' } } - const positionals = Array.isArray(configRecord._) ? configRecord._ as unknown[] : [] - for (const positional of positionals) { - const resolved = resolveCandidate(positional) - if (resolved && SUPPORTED_WDIO_CONFIG_EXTENSIONS.includes(path.extname(resolved))) { - return { configPath: resolved, strategy: 'config_positional' } - } - } - // `rootDir` defaults to dirname(configFile) but the user can override it in their // config, so it is a fallback and never the source of truth — try cwd as well. const rootDir = typeof configRecord.rootDir === 'string' ? configRecord.rootDir : undefined diff --git a/packages/browserstack-service/tests/configCapture.test.ts b/packages/browserstack-service/tests/configCapture.test.ts index 2817bca..8887452 100644 --- a/packages/browserstack-service/tests/configCapture.test.ts +++ b/packages/browserstack-service/tests/configCapture.test.ts @@ -150,6 +150,17 @@ describe('resolveWdioConfigPath', () => { expect(resolveWdioConfigPath({} as never)).toEqual({ reason: 'config_ambiguous' }) }) + it('does not mistake a --spec value for the config when there is no positional', () => { + // Regression: a `config._` rung would take the spec here, because `config._` carries + // the same argv WITHOUT the scan's flag-value guard. + const expected = write('wdio.conf.js') + write('a.js') + process.argv = ['node', 'wdio', '--spec', './a.js'] + + expect(resolveWdioConfigPath({ _: ['./a.js'] } as never)) + .toEqual({ configPath: expected, strategy: 'cwd_default' }) + }) + it('reports config_not_found on an empty project', () => { expect(resolveWdioConfigPath({} as never)).toEqual({ reason: 'config_not_found' }) }) From 812c85a7a5dac7ea02a48b29f5e43504635430c3 Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Thu, 13 Aug 2026 19:56:39 +0530 Subject: [PATCH 11/14] refactor(SDK-7250): resolve the config the way @wdio/cli does; keep manifest paths relative Review round 5. Both findings valid; the first also reverses my previous commit. 1. Config resolution now mirrors the CLI: one candidate, one stem, six extensions, no search. Verified with real yargs and wdio's own option declarations (spec: {type:'array'}, watch: {type:'boolean'}) that my last commit was wrong. I removed the `config._` rung after "proving" it would resolve a spec file, but I had hand-built `{_: ['./a.js']}` -- an input yargs cannot produce, because a declared array option consumes its value and leaves `_` EMPTY. The rung was never reachable that way. The inverse is real: `wdio --watch ./configs/a.conf.ts` leaves the config in `_[0]`, and the raw-argv scan skipped it by its own "previous token is a flag" rule. Confirmed against a live wdio run -- that invocation now resolves as config_positional, where at head it fell through to a directory guess. `config._` is the post-yargs value the CLI itself trusts; scanning raw argv re-implements yargs with a heuristic that cannot know which flags are boolean. Also fixes a case none of the old rungs could reach: the CLI hands Launcher the probed path but leaves `config-path` as the user's spelling, so a TS project legally carries `configs/a.conf.js` there while `configs/a.conf.ts` is on disk. Confirmed live -- wdio starts fine and reports the non-existent spelling. Stem-probing the CLI value resolves it deterministically, which is what makes the directory-scan rung unnecessary rather than load-bearing. Removed: isReadableFile-based resolveCandidate, probeConfigBasename, scanForSingleConfig, scanArgvForConfig and the ladder body. 347 -> 296 code lines. The `argv_positional` and `single_conf_scan` strategies and the `config_ambiguous` reason are gone. One test changed meaning rather than breaking: util.test.ts asserted no failure string on upload, which only held because the old directory scan was matching vitest.config.ts in the test cwd. It now correctly reports the soft `config_capture: config_not_found`, with success still true. 2. The capture manifest emitted an absolute project directory, reversing the decision made earlier in this PR to keep uploaded paths cwd-relative. relativeToCwd is now exported and used for both the manifest field and the debug line. Re-ran the live invocation matrix as asked: run form -> cli_config_path, bare form -> config_positional, no-arg -> root_dir_default, --watch form -> config_positional. Co-Authored-By: Claude Opus 5 (1M context) --- .../browserstack-service/src/configCapture.ts | 173 ++++++------------ packages/browserstack-service/src/util.ts | 8 +- .../tests/configCapture.test.ts | 72 ++++---- .../browserstack-service/tests/util.test.ts | 5 +- 4 files changed, 97 insertions(+), 161 deletions(-) diff --git a/packages/browserstack-service/src/configCapture.ts b/packages/browserstack-service/src/configCapture.ts index 7b103af..346c237 100644 --- a/packages/browserstack-service/src/configCapture.ts +++ b/packages/browserstack-service/src/configCapture.ts @@ -48,24 +48,32 @@ const isReadableFile = (filePath: string): boolean => { } /** - * WDIO hands us the config path exactly as the user typed it (relative or absolute), - * so every candidate is resolved against cwd — the same base the CLI itself uses - * (`create-wdio` formatConfigFilePaths). + * Accept a config path the way `@wdio/cli` does: the value as spelled, else the same stem + * probed across the supported extensions. + * + * The probe is not defensive padding — it is required. `commands/run.ts` hands `Launcher` the + * path that `canAccessConfigPath` found, but leaves `config-path` set to what the USER typed, + * and a TypeScript project legally spells a `.ts` config with `.js`. Verified against a real + * wdio run: `wdio run ./configs/a.conf.js` with only `configs/a.conf.ts` on disk starts fine + * and reports `config-path: './configs/a.conf.js'`, a path that does not exist. */ const resolveCandidate = (value: unknown): string | undefined => { - if (typeof value !== 'string' || value.trim() === '') { + if (typeof value !== 'string' || !value.trim()) { return undefined } try { - const resolved = path.resolve(process.cwd(), value.trim()) - return isReadableFile(resolved) ? resolved : undefined + const full = path.resolve(process.cwd(), value.trim()) + const ext = path.extname(full) + const stem = SUPPORTED_WDIO_CONFIG_EXTENSIONS.includes(ext) ? full.slice(0, -ext.length) : full + return [full, ...SUPPORTED_WDIO_CONFIG_EXTENSIONS.map((e) => `${stem}${e}`)] + .find(isReadableFile) } catch { return undefined } } -/** cwd-relative form of a path, for logs that get uploaded. Falls back to the input. */ -const relativeToCwd = (filePath: string): string => { +/** cwd-relative form of a path, for anything that gets uploaded. Falls back to the basename. */ +export const relativeToCwd = (filePath: string): string => { try { return path.relative(process.cwd(), filePath) || path.basename(filePath) } catch { @@ -73,135 +81,56 @@ const relativeToCwd = (filePath: string): string => { } } -const probeConfigBasename = (dir: string, basename: string): string | undefined => { - for (const ext of SUPPORTED_WDIO_CONFIG_EXTENSIONS) { - const candidate = path.join(dir, `${basename}${ext}`) - if (isReadableFile(candidate)) { - return candidate - } - } - return undefined -} - -/** - * Last-resort discovery: a directory containing exactly ONE `*.conf.` file is - * unambiguous. Two or more (e.g. `wdio.conf.ts` + `wdio.app.conf.ts`) is not, and we - * deliberately capture nothing rather than upload the wrong file. - */ -const scanForSingleConfig = (dir: string): { match?: string, ambiguous?: boolean } => { - try { - const matches = fs.readdirSync(dir) - .filter((entry) => SUPPORTED_WDIO_CONFIG_EXTENSIONS.includes(path.extname(entry))) - .filter((entry) => /\.conf(ig)?\.[^.]+$/i.test(entry)) - .map((entry) => path.join(dir, entry)) - .filter(isReadableFile) - - if (matches.length === 1) { - return { match: matches[0] } - } - return { ambiguous: matches.length > 1 } - } catch { - return {} - } -} - -/** - * Scan the raw CLI args for the config positional. - * - * Covers `wdio ` (bare form), where WDIO strips the path before the config - * object is built. A token is skipped when the PREVIOUS token is a flag, otherwise - * `wdio run conf.js --spec ./tests/a.js` would resolve to the spec file. - */ -const scanArgvForConfig = (argv: string[]): string | undefined => { - for (let i = 0; i < argv.length; i++) { - const arg = argv[i] - const previous = i > 0 ? argv[i - 1] : undefined - - if (!arg || arg.startsWith('-')) { - continue - } - if (WDIO_CLI_SUBCOMMANDS.includes(arg)) { - continue - } - // value of a space-separated flag (`--spec ./a.js`), not a positional - if (previous && previous.startsWith('-') && !previous.includes('=')) { - continue - } - if (!SUPPORTED_WDIO_CONFIG_EXTENSIONS.includes(path.extname(arg))) { - continue - } - - const resolved = resolveCandidate(arg) - if (resolved) { - return resolved - } - } - return undefined -} - /** * Resolve the absolute path of the user's wdio config file. * - * WDIO keeps the real path in `ConfigParser`'s private `#configFilePath` field, which no - * service can reach, so this walks a ladder of fallbacks — first rung that points at a - * file on disk wins: + * WDIO keeps the real path in `ConfigParser`'s private `#configFilePath`, which no service can + * reach, so this reconstructs it from the values the CLI does leave on the merged config — + * mirroring how `@wdio/cli` itself resolves it. The CLI never searches: it takes one candidate + * and probes one stem across the supported extensions, so neither does this. * * 1. BROWSERSTACK_WDIO_CONFIG_FILE_PATH — explicit override / support escape hatch * 2. config['config-path'] — yargs' kebab alias of the `run ` - * positional, which survives into the merged - * config object (v8 and v9 alike) - * 3. process.argv positional — `wdio ` without the `run` subcommand + * positional (v8 and v9 alike) + * 3. config._[0] — the bare `wdio ` form, which `run.ts` + * itself resolves from `params._[0]` * 4. rootDir + wdio.conf. — no-arg `wdio`, and programmatic `new Launcher()` * 5. cwd + wdio.conf. — when the user overrides `rootDir` in their config - * 6. single `*.conf.` in rootDir/cwd — unambiguous custom filenames only * - * There is deliberately NO `config._` rung. It looks like a free extra signal, but `config._` - * is derived from the same argv the scan above already reads — without the scan's guard that - * skips a flag's value. So it only ever differs by taking something the scan correctly - * rejected: `wdio --spec ./a.js` resolves to the SPEC file under that rung, and to the real - * `wdio.conf.js` without it. + * `config._` is read rather than raw `process.argv`: it is the same positional AFTER yargs has + * applied wdio's own option declarations. Scanning argv means re-implementing that with a + * heuristic that cannot know which flags are boolean — and `wdio --watch ./a.conf.ts` puts the + * real config in `_[0]` while any "skip a flag's value" rule throws it away. */ export function resolveWdioConfigPath(config?: Options.Testrunner): ConfigPathResolution { - const configRecord = (config || {}) as Record - - const fromEnv = resolveCandidate(process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH]) - if (fromEnv) { - return { configPath: fromEnv, strategy: 'env_override' } - } - - const fromConfigPath = resolveCandidate(configRecord['config-path']) - if (fromConfigPath) { - return { configPath: fromConfigPath, strategy: 'cli_config_path' } - } - - const fromArgv = scanArgvForConfig(process.argv.slice(2)) - if (fromArgv) { - return { configPath: fromArgv, strategy: 'argv_positional' } - } + const record = (config || {}) as Record - // `rootDir` defaults to dirname(configFile) but the user can override it in their - // config, so it is a fallback and never the source of truth — try cwd as well. - const rootDir = typeof configRecord.rootDir === 'string' ? configRecord.rootDir : undefined - const searchDirs = [rootDir, process.cwd()].filter((dir): dir is string => Boolean(dir)) - const uniqueDirs = Array.from(new Set(searchDirs)) - - for (const dir of uniqueDirs) { - const probed = probeConfigBasename(dir, DEFAULT_WDIO_CONFIG_BASENAME) - if (probed) { - return { configPath: probed, strategy: dir === rootDir ? 'root_dir_default' : 'cwd_default' } + try { + const positionals = Array.isArray(record._) ? record._ as unknown[] : [] + const positional = positionals.filter( + (entry): entry is string => typeof entry === 'string' && !WDIO_CLI_SUBCOMMANDS.includes(entry) + )[0] + const rootDir = typeof record.rootDir === 'string' ? record.rootDir : undefined + + const rungs: Array<[string, unknown]> = [ + ['env_override', process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH]], + ['cli_config_path', record['config-path']], + ['config_positional', positional], + ['root_dir_default', rootDir && path.join(rootDir, DEFAULT_WDIO_CONFIG_BASENAME)], + ['cwd_default', DEFAULT_WDIO_CONFIG_BASENAME] + ] + + for (const [strategy, value] of rungs) { + const configPath = resolveCandidate(value) + if (configPath) { + return { configPath, strategy } + } } - } - let sawAmbiguous = false - for (const dir of uniqueDirs) { - const { match, ambiguous } = scanForSingleConfig(dir) - if (match) { - return { configPath: match, strategy: 'single_conf_scan' } - } - sawAmbiguous = sawAmbiguous || Boolean(ambiguous) + return { reason: 'config_not_found' } + } catch { + return { reason: 'config_resolve_exception' } } - - return { reason: sawAmbiguous ? 'config_ambiguous' : 'config_not_found' } } /** diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index a63df0e..d35a7d3 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -48,7 +48,7 @@ import { } from './constants.js' import CrashReporter from './crash-reporter.js' import { BStackLogger } from './bstackLogger.js' -import { collectConfigFilesForUpload, dedupeEntryName, findPackageJsonForUpload, isAutoCaptureLogsDisabled, redactSensitiveContent } from './configCapture.js' +import { collectConfigFilesForUpload, dedupeEntryName, findPackageJsonForUpload, isAutoCaptureLogsDisabled, redactSensitiveContent, relativeToCwd } from './configCapture.js' import UsageStats from './testOps/usageStats.js' import TestOpsConfig from './testOps/testOpsConfig.js' import type { StartBinSessionResponse } from './grpc/index.js' @@ -1632,7 +1632,7 @@ export async function uploadLogs(user: string | undefined, key: string | undefin BStackLogger.debug(`No wdio config captured${strategy ? ` (strategy ${strategy})` : ''}`) } if (packageJsonPath) { - BStackLogger.debug(`Auto-captured package.json from ${path.dirname(packageJsonPath)}`) + BStackLogger.debug(`Auto-captured package.json from ${relativeToCwd(path.dirname(packageJsonPath))}`) } if (configFailures.length > 0 && failure === undefined) { // Warning only — `success` stays true so a missing config never reads as a @@ -1655,7 +1655,9 @@ export async function uploadLogs(user: string | undefined, key: string | undefin `archive entries: ${[...copiedFileNames, manifestName].join(', ')}`, `config resolution strategy: ${strategy || 'none'}`, `config files captured: ${capturedConfigNames.join(', ') || 'none'}`, - `package.json: ${packageJsonPath ? path.dirname(packageJsonPath) : 'not found'}`, + // cwd-relative: the manifest ships inside the tarball, so an absolute path would + // leak the OS username and layout — the same reason the resolution log line is relative + `package.json: ${packageJsonPath ? relativeToCwd(path.dirname(packageJsonPath)) : 'not found'}`, `capture failures: ${configFailures.join('; ') || 'none'}` ] fs.writeFileSync(path.join(tmpDir, manifestName), manifestLines.join('\n') + '\n') diff --git a/packages/browserstack-service/tests/configCapture.test.ts b/packages/browserstack-service/tests/configCapture.test.ts index 8887452..6c81b6c 100644 --- a/packages/browserstack-service/tests/configCapture.test.ts +++ b/packages/browserstack-service/tests/configCapture.test.ts @@ -95,29 +95,48 @@ describe('resolveWdioConfigPath', () => { .toEqual({ configPath: expected, strategy: 'cwd_default' }) }) - it('falls back to the argv positional for the bare `wdio ` form', () => { + it('resolves the bare `wdio ` form from config._', () => { const expected = write('custom.conf.mjs') - process.argv = ['node', 'wdio', './custom.conf.mjs'] - expect(resolveWdioConfigPath({} as never)) - .toEqual({ configPath: expected, strategy: 'argv_positional' }) + expect(resolveWdioConfigPath({ _: ['./custom.conf.mjs'] } as never)) + .toEqual({ configPath: expected, strategy: 'config_positional' }) }) - it('does not mistake a --spec value for the config positional', () => { - const expected = write('wdio.conf.ts') - write('test/login.e2e.js') - process.argv = ['node', 'wdio', 'run', './wdio.conf.ts', '--spec', './test/login.e2e.js'] + it('resolves `wdio --watch `, where the config lands in _ after a boolean flag', () => { + // yargs consumes nothing for a boolean flag, so `_[0]` IS the config. A raw-argv scan + // that skips "a flag's value" throws this away; reading config._ cannot. + const expected = write('configs/a.conf.ts') + + expect(resolveWdioConfigPath({ _: ['./configs/a.conf.ts'], watch: true } as never)) + .toEqual({ configPath: expected, strategy: 'config_positional' }) + }) - expect(resolveWdioConfigPath({} as never)) - .toEqual({ configPath: expected, strategy: 'argv_positional' }) + it('probes the stem when the CLI value carries a .js spelling for a .ts file', () => { + // Real wdio behaviour: `wdio run ./configs/a.conf.js` starts fine with only + // configs/a.conf.ts on disk, and leaves the non-existent spelling on config-path. + const expected = write('configs/a.conf.ts') + + expect(resolveWdioConfigPath({ 'config-path': './configs/a.conf.js' } as never)) + .toEqual({ configPath: expected, strategy: 'cli_config_path' }) + }) + + it('cannot be fooled by a --spec value, because yargs never leaves it in _', () => { + // Measured with wdio's own declaration (spec: {type:'array'}): `wdio --spec ./a.js` + // leaves _ EMPTY, so there is no positional to mistake, and resolution falls through + // to the default config. This is why reading config._ needs no flag-value heuristic. + const expected = write('wdio.conf.js') + write('a.js') + + expect(resolveWdioConfigPath({ _: [], spec: ['./a.js'] } as never)) + .toEqual({ configPath: expected, strategy: 'cwd_default' }) }) - it('skips the `run` subcommand when scanning argv', () => { + it('ignores the `run` subcommand when reading positionals', () => { write('run') const expected = write('wdio.conf.cts') - process.argv = ['node', 'wdio', 'run', './wdio.conf.cts'] - expect(resolveWdioConfigPath({} as never).configPath).toBe(expected) + expect(resolveWdioConfigPath({ _: ['run', './wdio.conf.cts'] } as never).configPath) + .toBe(expected) }) it('probes rootDir for wdio.conf with every supported extension', () => { @@ -136,29 +155,12 @@ describe('resolveWdioConfigPath', () => { .toEqual({ configPath: expected, strategy: 'cwd_default' }) }) - it('accepts a single custom *.conf.* file as unambiguous', () => { - const expected = write('e2e.conf.ts') + it('does not guess when a custom-named config exists and the CLI gave nothing', () => { + // The directory-scan rung is gone: wdio itself never searches, so neither do we. + // Stem-probing the CLI value covers what that rung used to rescue, deterministically. + write('e2e.conf.ts') - expect(resolveWdioConfigPath({} as never)) - .toEqual({ configPath: expected, strategy: 'single_conf_scan' }) - }) - - it('captures nothing when several custom configs are present', () => { - write('android.conf.ts') - write('ios.conf.ts') - - expect(resolveWdioConfigPath({} as never)).toEqual({ reason: 'config_ambiguous' }) - }) - - it('does not mistake a --spec value for the config when there is no positional', () => { - // Regression: a `config._` rung would take the spec here, because `config._` carries - // the same argv WITHOUT the scan's flag-value guard. - const expected = write('wdio.conf.js') - write('a.js') - process.argv = ['node', 'wdio', '--spec', './a.js'] - - expect(resolveWdioConfigPath({ _: ['./a.js'] } as never)) - .toEqual({ configPath: expected, strategy: 'cwd_default' }) + expect(resolveWdioConfigPath({} as never)).toEqual({ reason: 'config_not_found' }) }) it('reports config_not_found on an empty project', () => { diff --git a/packages/browserstack-service/tests/util.test.ts b/packages/browserstack-service/tests/util.test.ts index af292e2..03d1109 100644 --- a/packages/browserstack-service/tests/util.test.ts +++ b/packages/browserstack-service/tests/util.test.ts @@ -1676,10 +1676,13 @@ describe('uploadLogs', function () { it('should upload the logs', async function () { await uploadLogs('some_user', 'some_key', 'some_uuid') expect(fetch).toHaveBeenCalled() + // The suite runs with no resolvable wdio config, so config capture records the soft + // `config_not_found` warning. `success` staying true is the contract that matters: + // a missing config must never read as a failed log upload. expect(endSpy).toHaveBeenCalledWith( PERFORMANCE_SDK_EVENTS.EVENTS.SDK_UPLOAD_LOGS, true, - undefined + 'config_capture: config_not_found' ) }) From 927c83773fd4cfa60d4bd9848539d2df448587b4 Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Thu, 13 Aug 2026 20:31:32 +0530 Subject: [PATCH 12/14] fix(SDK-7250): render a project-root manifest dir as "." instead of its folder name relativeToCwd is `path.relative(...) || path.basename(...)`, and `path.relative(cwd, cwd)` is ''. That fallback was written for FILE paths, where the result is never empty. The manifest passes a DIRECTORY, so the common case -- a package.json at the project root -- hit the fallback and printed the folder name: manifest at cwd -> "package.json: my-e2e-project" project checked out in $HOME -> "package.json: jane.doe" <- OS username, in the tarball manifest above cwd -> "package.json: ../.." <- correct Row 2 is the same exposure the relative-path handling exists to prevent, and row 1 silently reports a folder name where the reader expects ".". Added relativeDirToCwd, which renders empty as "." because for a directory empty MEANS cwd, and used it at both call sites. relativeToCwd keeps its file semantics, with a note not to pass a directory to it. Live-verified: the debug line now reads "Auto-captured package.json from ." with the cwd basename absent. Also rewrote the "leaves no staging directory behind" test, which was flaky for a reason unrelated to this review: it diffed a listing of os.tmpdir(), and vitest runs test FILES in parallel workers where util.test.ts also calls uploadLogs, so it raced against staging dirs another worker was creating and removing. It now spies on mkdtempSync and asserts the specific directories that this call created are gone. Co-Authored-By: Claude Opus 5 (1M context) --- .../browserstack-service/src/configCapture.ts | 23 +++++++++++++- packages/browserstack-service/src/util.ts | 6 ++-- .../tests/configCapture.test.ts | 19 ++++++++++++ .../tests/uploadLogsArchive.test.ts | 31 +++++++++++++++++-- 4 files changed, 72 insertions(+), 7 deletions(-) diff --git a/packages/browserstack-service/src/configCapture.ts b/packages/browserstack-service/src/configCapture.ts index 346c237..5b56e88 100644 --- a/packages/browserstack-service/src/configCapture.ts +++ b/packages/browserstack-service/src/configCapture.ts @@ -72,7 +72,12 @@ const resolveCandidate = (value: unknown): string | undefined => { } } -/** cwd-relative form of a path, for anything that gets uploaded. Falls back to the basename. */ +/** + * cwd-relative form of a FILE path, for anything that gets uploaded. + * + * The basename fallback is safe here only because `path.relative` is never empty for a file — + * a file is never equal to cwd. Do NOT pass a directory: see `relativeDirToCwd`. + */ export const relativeToCwd = (filePath: string): string => { try { return path.relative(process.cwd(), filePath) || path.basename(filePath) @@ -81,6 +86,22 @@ export const relativeToCwd = (filePath: string): string => { } } +/** + * cwd-relative form of a DIRECTORY path. + * + * `path.relative(cwd, cwd)` is `''`, which is the COMMON case here (a manifest at the project + * root), so a basename fallback would report the folder name — and for a project checked out + * directly in `$HOME` that folder name is the OS username, which is the exact exposure the + * relative-path handling exists to prevent. Empty means "cwd", so render it as `.`. + */ +export const relativeDirToCwd = (dir: string): string => { + try { + return path.relative(process.cwd(), dir) || '.' + } catch { + return '.' + } +} + /** * Resolve the absolute path of the user's wdio config file. * diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index d35a7d3..c188088 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -48,7 +48,7 @@ import { } from './constants.js' import CrashReporter from './crash-reporter.js' import { BStackLogger } from './bstackLogger.js' -import { collectConfigFilesForUpload, dedupeEntryName, findPackageJsonForUpload, isAutoCaptureLogsDisabled, redactSensitiveContent, relativeToCwd } from './configCapture.js' +import { collectConfigFilesForUpload, dedupeEntryName, findPackageJsonForUpload, isAutoCaptureLogsDisabled, redactSensitiveContent, relativeDirToCwd } from './configCapture.js' import UsageStats from './testOps/usageStats.js' import TestOpsConfig from './testOps/testOpsConfig.js' import type { StartBinSessionResponse } from './grpc/index.js' @@ -1632,7 +1632,7 @@ export async function uploadLogs(user: string | undefined, key: string | undefin BStackLogger.debug(`No wdio config captured${strategy ? ` (strategy ${strategy})` : ''}`) } if (packageJsonPath) { - BStackLogger.debug(`Auto-captured package.json from ${relativeToCwd(path.dirname(packageJsonPath))}`) + BStackLogger.debug(`Auto-captured package.json from ${relativeDirToCwd(path.dirname(packageJsonPath))}`) } if (configFailures.length > 0 && failure === undefined) { // Warning only — `success` stays true so a missing config never reads as a @@ -1657,7 +1657,7 @@ export async function uploadLogs(user: string | undefined, key: string | undefin `config files captured: ${capturedConfigNames.join(', ') || 'none'}`, // cwd-relative: the manifest ships inside the tarball, so an absolute path would // leak the OS username and layout — the same reason the resolution log line is relative - `package.json: ${packageJsonPath ? relativeToCwd(path.dirname(packageJsonPath)) : 'not found'}`, + `package.json: ${packageJsonPath ? relativeDirToCwd(path.dirname(packageJsonPath)) : 'not found'}`, `capture failures: ${configFailures.join('; ') || 'none'}` ] fs.writeFileSync(path.join(tmpDir, manifestName), manifestLines.join('\n') + '\n') diff --git a/packages/browserstack-service/tests/configCapture.test.ts b/packages/browserstack-service/tests/configCapture.test.ts index 6c81b6c..ab523a2 100644 --- a/packages/browserstack-service/tests/configCapture.test.ts +++ b/packages/browserstack-service/tests/configCapture.test.ts @@ -5,6 +5,7 @@ import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' import { collectConfigFilesForUpload, + relativeDirToCwd, findPackageJsonForUpload, initWdioConfigPath, isAutoCaptureLogsDisabled, @@ -521,6 +522,24 @@ describe('collectConfigFilesForUpload', () => { }) }) +describe('relativeDirToCwd', () => { + it('renders the cwd itself as "." and never as its folder name', () => { + // path.relative(cwd, cwd) === '', which is the COMMON case for a project-root + // manifest. A basename fallback would print the folder name — and for a project + // checked out directly in $HOME that folder name is the OS username. + expect(relativeDirToCwd(tmpRoot)).toBe('.') + expect(relativeDirToCwd(tmpRoot)).not.toBe(path.basename(tmpRoot)) + }) + + it('keeps the diagnostic for a directory outside cwd', () => { + expect(relativeDirToCwd(path.dirname(tmpRoot))).toBe('..') + }) + + it('is relative for a nested directory', () => { + expect(relativeDirToCwd(path.join(tmpRoot, 'packages', 'e2e'))).toBe(path.join('packages', 'e2e')) + }) +}) + describe('findPackageJsonForUpload', () => { it('prefers the package.json next to the resolved config', () => { const expected = write('project/package.json', '{"name":"app"}') diff --git a/packages/browserstack-service/tests/uploadLogsArchive.test.ts b/packages/browserstack-service/tests/uploadLogsArchive.test.ts index 43375aa..96bb044 100644 --- a/packages/browserstack-service/tests/uploadLogsArchive.test.ts +++ b/packages/browserstack-service/tests/uploadLogsArchive.test.ts @@ -5,6 +5,8 @@ import zlib from 'node:zlib' import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' import { list } from 'tar' +const fsMkdtempOriginal = fs.mkdtempSync + import { uploadLogs } from '../src/util.js' import { BStackLogger } from '../src/bstackLogger.js' import { BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS, BROWSERSTACK_WDIO_CONFIG_FILE_PATH } from '../src/constants.js' @@ -140,6 +142,18 @@ describe('uploadLogs archive contents (SDK-7250)', () => { expect(raw).toContain('wdio.conf.js') }) + it('reports a project-root manifest as "." rather than the folder name', async () => { + fs.writeFileSync(path.join(tmpProject, 'package.json'), '{"name":"app"}') + fs.writeFileSync(path.join(tmpProject, 'wdio.conf.js'), 'export const config = {}') + + await uploadLogs('some_user', 'some_key', 'some_uuid', {}) + + const raw = zlib.gunzipSync(uploadedArchive!).toString('binary') + expect(raw).toContain('package.json: .') + // the tmp dir's own name must not leak into the bundle + expect(raw).not.toContain(`package.json: ${path.basename(tmpProject)}`) + }) + it('records the reason in the manifest when no config is found', async () => { await uploadLogs('some_user', 'some_key', 'some_uuid', {}) @@ -178,13 +192,24 @@ describe('uploadLogs archive contents (SDK-7250)', () => { }) it('leaves no staging directory behind', async () => { + // Spy on the creation rather than diffing os.tmpdir(): vitest runs test FILES in + // parallel workers and util.test.ts also calls uploadLogs, so a listing-based check + // races against staging dirs another worker is creating and removing. fs.writeFileSync(path.join(tmpProject, 'wdio.conf.js'), 'export const config = {}') - const before = fs.readdirSync(os.tmpdir()).filter((e) => e.startsWith('bstack-wdio-logs-')) + const created: string[] = [] + const mkdtempSpy = vi.spyOn(fs, 'mkdtempSync').mockImplementation(((prefix: string) => { + const dir = fsMkdtempOriginal(prefix) as string + created.push(dir) + return dir + }) as never) await uploadLogs('some_user', 'some_key', 'some_uuid', {}) + mkdtempSpy.mockRestore() - const after = fs.readdirSync(os.tmpdir()).filter((e) => e.startsWith('bstack-wdio-logs-')) - expect(after).toEqual(before) + expect(created.length).toBeGreaterThan(0) + for (const dir of created) { + expect(fs.existsSync(dir)).toBe(false) + } }) it('keeps concurrent runs from clobbering each other', async () => { From 22946ba76e2d7f9a7e046487fc77f449579b5f98 Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Thu, 13 Aug 2026 21:12:35 +0530 Subject: [PATCH 13/14] fix(SDK-7250): only follow imported files that are themselves configs The import follower resolved any relative specifier landing on a supported extension, with no name filter. So a config doing `import { helper } from './helpers/utils.js'` had that module captured and uploaded too -- ordinary application source, not configuration. Confirmed on a real run before this change: the archive carried `utils.js`. Capturing the split-config case is the point of following imports at all; shipping a customer's application modules is not. Imports are now followed only when the resolved file is named like a config (`*.conf.*` / `*.config.*`), which keeps `base.conf.js` and `wdio.shared.conf.ts` and drops everything else. Verified on the same fixture, with the entry config still importing the helper: Auto-captured 2 config file(s) via cli_config_path: wdio.bstack.conf.js, base.conf.js archive entries: bstack-wdio-service.log, sdk-cli-debug.log, wdio.bstack.conf.js, base.conf.js, package.json, capture-manifest.txt This also makes the user-facing wording true as written: the changeset, the JSDoc and the runtime notice all say "the local config files it imports", which was inaccurate until now. Co-Authored-By: Claude Opus 5 (1M context) --- .../browserstack-service/src/configCapture.ts | 16 ++++++++++--- .../tests/configCapture.test.ts | 24 +++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/packages/browserstack-service/src/configCapture.ts b/packages/browserstack-service/src/configCapture.ts index 5b56e88..521560b 100644 --- a/packages/browserstack-service/src/configCapture.ts +++ b/packages/browserstack-service/src/configCapture.ts @@ -303,11 +303,21 @@ const readCappedFile = (filePath: string): { content?: string, reason?: string } * Handles the TypeScript-ESM convention where `./shared.conf.js` on disk is actually * `./shared.conf.ts`, plus extension-less and directory (`/index.*`) specifiers. */ +/** + * Only files that are themselves configs are followed. + * + * A relative specifier resolves to whatever the config happens to import, which is frequently + * ordinary application source (`./helpers/utils.js`) rather than configuration. Capturing the + * split-config case (`base.conf.js`, `wdio.shared.conf.ts`) is the point; shipping a + * customer's application modules is not. + */ +const isConfigFileName = (filePath: string): boolean => /\.conf(ig)?\.[^.]+$/i.test(path.basename(filePath)) + const resolveRelativeImport = (specifier: string, fromFile: string): string | undefined => { const base = path.resolve(path.dirname(fromFile), specifier) if (isReadableFile(base) && SUPPORTED_WDIO_CONFIG_EXTENSIONS.includes(path.extname(base))) { - return base + return isConfigFileName(base) ? base : undefined } const withoutExt = SUPPORTED_WDIO_CONFIG_EXTENSIONS.includes(path.extname(base)) @@ -316,13 +326,13 @@ const resolveRelativeImport = (specifier: string, fromFile: string): string | un for (const ext of SUPPORTED_WDIO_CONFIG_EXTENSIONS) { const candidate = `${withoutExt}${ext}` - if (isReadableFile(candidate)) { + if (isReadableFile(candidate) && isConfigFileName(candidate)) { return candidate } } for (const ext of SUPPORTED_WDIO_CONFIG_EXTENSIONS) { const candidate = path.join(base, `index${ext}`) - if (isReadableFile(candidate)) { + if (isReadableFile(candidate) && isConfigFileName(candidate)) { return candidate } } diff --git a/packages/browserstack-service/tests/configCapture.test.ts b/packages/browserstack-service/tests/configCapture.test.ts index ab523a2..0224db9 100644 --- a/packages/browserstack-service/tests/configCapture.test.ts +++ b/packages/browserstack-service/tests/configCapture.test.ts @@ -481,6 +481,30 @@ describe('collectConfigFilesForUpload', () => { expect(files.map((f) => f.content).join('\n')).not.toContain('leaked-from-import') }) + it('follows an imported CONFIG file but not ordinary source', () => { + write('wdio.conf.ts', [ + "import { base } from './base.conf.js'", + "import { helper } from './helpers/utils.js'", + 'export const config = {}' + ].join('\n')) + write('base.conf.ts', 'export const base = { maxInstances: 5 }') + write('helpers/utils.js', 'export const helper = 1 // ORDINARY_SOURCE') + + const { files } = collectConfigFilesForUpload({} as never) + + expect(files.map((f) => f.name).sort()).toEqual(['base.conf.ts', 'wdio.conf.ts']) + expect(files.map((f) => f.content).join('\n')).not.toContain('ORDINARY_SOURCE') + }) + + it('does not follow a config-shaped path that is not named like a config', () => { + write('wdio.conf.ts', "import './lib/setup.ts'\nexport const config = {}") + write('lib/setup.ts', 'export const x = 1 // NOT_A_CONFIG') + + const { files } = collectConfigFilesForUpload({} as never) + + expect(files.map((f) => f.name)).toEqual(['wdio.conf.ts']) + }) + it('never follows bare (npm package) specifiers', () => { write('wdio.conf.ts', 'import { x } from "@wdio/globals"\nimport y from "dotenv"\nexport const config = {}') From b8d39c13211e3fc84a157e6d95316318d3b75573 Mon Sep 17 00:00:00 2001 From: Aakash Hotchandani Date: Thu, 13 Aug 2026 21:41:18 +0530 Subject: [PATCH 14/14] fix(SDK-7250): drop capture-manifest.txt; put the capture summary in the shipped log The manifest existed only because the capture lines never reached the archive: the service log was copyFileSync'd into the staging dir before those lines were written, so they lived only in the developer's local file. Rather than ship a second file, take the reviewer's first suggested option and fix the ordering. Ordering alone was not sufficient, which a downloaded bundle proved: BStackLogger writes through an async fs.WriteStream, so the summary lines were still buffered when the copy ran and the shipped log was silently truncated. Adds BStackLogger.flushLogFile() -- a zero-length write whose callback fires after every queued chunk reaches the fs layer, draining the buffer without ending the stream, and bounded by a timeout so a stuck stream can never hold up the upload. The trailing "archive entries" line stays local-only by design: it lists what actually landed, and the log file is itself one of those entries. Verified end-to-end on build xbpsytnxggzx6kp03mjt0j4zptx4hs9ece2kocv7 -- the downloaded bundle carries no capture-manifest.txt, and its bstack-wdio-service.log contains the strategy, the captured config names and the package.json origin. 0 credential hits across every file in the bundle. Tests: both fixes are independently mutation-checked (removing either the flush or the reorder fails the suite), plus direct tests for flushLogFile. 1149 tests green. Co-Authored-By: Claude Opus 5 (1M context) --- .../browserstack-service/src/bstackLogger.ts | 28 ++++ packages/browserstack-service/src/util.ts | 64 ++++----- .../tests/uploadLogsArchive.test.ts | 129 ++++++++++++++---- 3 files changed, 157 insertions(+), 64 deletions(-) diff --git a/packages/browserstack-service/src/bstackLogger.ts b/packages/browserstack-service/src/bstackLogger.ts index b397fd8..e33c121 100644 --- a/packages/browserstack-service/src/bstackLogger.ts +++ b/packages/browserstack-service/src/bstackLogger.ts @@ -73,6 +73,34 @@ export class BStackLogger { log.trace(redactedMessage) } + /** + * Drain whatever is still sitting in the log stream's buffer onto disk. + * + * `logToFile` writes to an async `fs.WriteStream`, so a line logged immediately before the + * archive is built is very likely NOT in the file yet — the stream's `open` is async too, + * so early on the file may not exist at all. Anything that snapshots the log (the debug-log + * upload) must flush first or it ships a truncated copy. + * + * A zero-length write's callback fires only after every chunk queued ahead of it has been + * handed to the fs layer, which drains the buffer without ending the stream — unlike + * `clearLogger()`, logging continues to work afterwards. + */ + public static async flushLogFile(timeoutMs = 2000): Promise { + const stream = this.logFileStream + if (!stream || !stream.writable) { + return + } + // Never let a stuck stream hold up the upload; a truncated log beats no log at all. + await new Promise((resolve) => { + const timer = setTimeout(resolve, timeoutMs) + timer.unref?.() + stream.write('', () => { + clearTimeout(timer) + resolve() + }) + }) + } + public static clearLogger() { if (this.logFileStream) { this.logFileStream.end() diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index c188088..8012f07 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -1567,25 +1567,9 @@ export async function uploadLogs(user: string | undefined, key: string | undefin // (e.g. configs/wdio.conf.ts + shared/wdio.conf.ts) join the archive. // Same helper collectConfigFilesForUpload uses, so the two cannot disagree. const takenNames = new Set(['logs.tar', 'logs.tar.gz']) - const uniqueName = (filePath: string): string => dedupeEntryName(filePath, takenNames) - - const filesToArchive = [ - BStackLogger.logFilePath, - CLI_DEBUG_LOGS_FILE, - ].filter((f): f is string => Boolean(f) && fs.existsSync(f as string)) - const copiedFileNames: string[] = [] const archiveAddFailures: string[] = [] - for (const f of filesToArchive) { - try { - const entryName = uniqueName(f) - fs.copyFileSync(f, path.join(tmpDir, entryName)) - copiedFileNames.push(entryName) - } catch (copyErr) { - const msg = (copyErr as Error)?.message || String(copyErr) - archiveAddFailures.push(`${path.basename(f)}: ${msg}`) - } - } + const uniqueName = (filePath: string): string => dedupeEntryName(filePath, takenNames) // SDK-7250: the user's wdio config (and the local files it imports), credential-scrubbed. // Soft-failure by design — a config we cannot read or locate must never stop the @@ -1640,31 +1624,37 @@ export async function uploadLogs(user: string | undefined, key: string | undefin failure = `config_capture: ${configFailures.join('; ')}`.substring(0, 300) } + // The service log is snapshotted LAST, deliberately. Everything above -- the resolution + // strategy, what was captured, and any capture failures -- is written to that log + // first, so it is inside the copy that ships. Copying earlier is why those lines used + // to exist only in the developer's local file and never in the downloaded bundle. + // + // Ordering alone is not enough: BStackLogger writes through an async WriteStream, so + // those lines are still buffered at this point. Flush before copying or the shipped + // copy is silently missing its tail (verified against a downloaded bundle). + await BStackLogger.flushLogFile() + + const filesToArchive = [ + BStackLogger.logFilePath, + CLI_DEBUG_LOGS_FILE, + ].filter((f): f is string => Boolean(f) && fs.existsSync(f as string)) + + for (const f of filesToArchive) { + try { + const entryName = uniqueName(f) + fs.copyFileSync(f, path.join(tmpDir, entryName)) + copiedFileNames.push(entryName) + } catch (copyErr) { + const msg = (copyErr as Error)?.message || String(copyErr) + archiveAddFailures.push(`${path.basename(f)}: ${msg}`) + } + } + if (archiveAddFailures.length > 0) { success = false failure = `archive_add_failed [${archiveAddFailures.length}]: ${archiveAddFailures.join('; ')}`.substring(0, 300) } - // Written as its OWN archive entry rather than only to the service log. The log file - // is snapshotted into the staging dir above, so anything logged after that copy never - // reaches the tarball — support downloading the bundle would see no manifest, no - // resolution strategy and no capture failures. Those are exactly where triage starts. - try { - const manifestName = uniqueName('capture-manifest.txt') - const manifestLines = [ - `archive entries: ${[...copiedFileNames, manifestName].join(', ')}`, - `config resolution strategy: ${strategy || 'none'}`, - `config files captured: ${capturedConfigNames.join(', ') || 'none'}`, - // cwd-relative: the manifest ships inside the tarball, so an absolute path would - // leak the OS username and layout — the same reason the resolution log line is relative - `package.json: ${packageJsonPath ? relativeDirToCwd(path.dirname(packageJsonPath)) : 'not found'}`, - `capture failures: ${configFailures.join('; ') || 'none'}` - ] - fs.writeFileSync(path.join(tmpDir, manifestName), manifestLines.join('\n') + '\n') - copiedFileNames.push(manifestName) - } catch (manifestErr) { - BStackLogger.debug(`Failed to write capture manifest: ${getErrorString(manifestErr)}`) - } BStackLogger.debug(`Auto-capture archive entries: ${copiedFileNames.join(', ')}`) await create( diff --git a/packages/browserstack-service/tests/uploadLogsArchive.test.ts b/packages/browserstack-service/tests/uploadLogsArchive.test.ts index 96bb044..91b9184 100644 --- a/packages/browserstack-service/tests/uploadLogsArchive.test.ts +++ b/packages/browserstack-service/tests/uploadLogsArchive.test.ts @@ -20,6 +20,26 @@ let uploadedArchive: Buffer | undefined let tmpProject: string let cwdSpy: ReturnType let originalLogFilePath: string +let originalLogFolderPath: string + +const readArchiveEntry = async (gz: Buffer, entryName: string): Promise => { + const workDir = fs.mkdtempSync(path.join(os.tmpdir(), 'bstack-untar-one-')) + const tarPath = path.join(workDir, 'logs.tar') + fs.writeFileSync(tarPath, zlib.gunzipSync(gz)) + + let content = '' + await list({ + file: tarPath, + onentry: (e) => { + if (String(e.path) !== entryName) { + return + } + e.on('data', (chunk: Buffer) => { content += chunk.toString('utf8') }) + } + }) + fs.rmSync(workDir, { recursive: true, force: true }) + return content +} const readArchiveEntries = async (gz: Buffer) => { const workDir = fs.mkdtempSync(path.join(os.tmpdir(), 'bstack-untar-')) @@ -37,6 +57,11 @@ beforeEach(() => { tmpProject = fs.mkdtempSync(path.join(os.tmpdir(), 'bstack-upload-test-')) cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(tmpProject) originalLogFilePath = BStackLogger.logFilePath + originalLogFolderPath = BStackLogger.logFolderPath + // BStackLogger caches its WriteStream on a static; drop it so it reopens at THIS test's + // path. Without this the stream still points at the previous test's (deleted) directory. + BStackLogger.clearLogger() + BStackLogger.logFolderPath = tmpProject BStackLogger.logFilePath = path.join(tmpProject, 'bstack-wdio-service.log') fs.writeFileSync(BStackLogger.logFilePath, 'service log content') delete process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] @@ -55,7 +80,9 @@ beforeEach(() => { afterEach(() => { cwdSpy.mockRestore() + BStackLogger.clearLogger() BStackLogger.logFilePath = originalLogFilePath + BStackLogger.logFolderPath = originalLogFolderPath delete process.env[BROWSERSTACK_WDIO_CONFIG_FILE_PATH] delete process.env[BROWSERSTACK_DISABLE_AUTO_CAPTURE_LOGS] fs.rmSync(tmpProject, { recursive: true, force: true }) @@ -125,41 +152,30 @@ describe('uploadLogs archive contents (SDK-7250)', () => { expect(raw).toContain('accessibility: true') }) - it('puts the capture manifest INSIDE the archive, not just in the log', async () => { - // The service log is snapshotted into the staging dir before these lines are written, - // so anything logged afterwards never reaches the tarball. Support downloading the - // bundle needs the manifest, the strategy and the failures to be in it. + it('puts the capture summary inside the ARCHIVED log, not just the local one', async () => { + // Two separate regressions are pinned here, both found against a real downloaded + // bundle. (1) The log is snapshotted LAST, so the resolution strategy and captured + // names are inside the copy that ships -- copying earlier left them local-only. + // (2) BStackLogger writes through an async WriteStream, so ordering alone still + // shipped a truncated log; uploadLogs must flush before it copies. fs.writeFileSync(path.join(tmpProject, 'package.json'), '{"name":"app"}') fs.writeFileSync(path.join(tmpProject, 'wdio.conf.js'), 'export const config = {}') - await uploadLogs('some_user', 'some_key', 'some_uuid', {}) - const entries = await readArchiveEntries(uploadedArchive!) - expect(entries).toContain('capture-manifest.txt') - - const raw = zlib.gunzipSync(uploadedArchive!).toString('binary') - expect(raw).toContain('config resolution strategy:') - expect(raw).toContain('wdio.conf.js') + const archivedLog = await readArchiveEntry(uploadedArchive!, 'bstack-wdio-service.log') + expect(archivedLog).toContain('Auto-captured 1 config file(s) via cwd_default') + expect(archivedLog).toContain('wdio.conf.js') + expect(archivedLog).toContain('Auto-captured package.json from') + // The trailing "archive entries" line lists what actually landed, so it can only be + // written after the copy -- the log file is itself one of those entries. It stays + // local-only on purpose; whoever holds the bundle can just list the tarball. }) - it('reports a project-root manifest as "." rather than the folder name', async () => { - fs.writeFileSync(path.join(tmpProject, 'package.json'), '{"name":"app"}') - fs.writeFileSync(path.join(tmpProject, 'wdio.conf.js'), 'export const config = {}') - + it('records in the ARCHIVED log when no config was found', async () => { await uploadLogs('some_user', 'some_key', 'some_uuid', {}) - const raw = zlib.gunzipSync(uploadedArchive!).toString('binary') - expect(raw).toContain('package.json: .') - // the tmp dir's own name must not leak into the bundle - expect(raw).not.toContain(`package.json: ${path.basename(tmpProject)}`) - }) - - it('records the reason in the manifest when no config is found', async () => { - await uploadLogs('some_user', 'some_key', 'some_uuid', {}) - - const raw = zlib.gunzipSync(uploadedArchive!).toString('binary') - expect(raw).toContain('capture failures:') - expect(raw).toContain('config files captured: none') + const archivedLog = await readArchiveEntry(uploadedArchive!, 'bstack-wdio-service.log') + expect(archivedLog).toContain('No wdio config captured') }) it('still uploads the logs when no config can be found', async () => { @@ -225,3 +241,62 @@ describe('uploadLogs archive contents (SDK-7250)', () => { expect(fetch).toHaveBeenCalledTimes(3) }) }) + +describe('BStackLogger.flushLogFile', () => { + // Lives here rather than in bstackLogger.test.ts, which mocks node:fs wholesale -- + // buffering is the behaviour under test, so it needs a real stream and a real file. + let dir: string + let originalPath: string + let originalFolder: string + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'bstack-flush-')) + originalPath = BStackLogger.logFilePath + originalFolder = BStackLogger.logFolderPath + BStackLogger.clearLogger() + BStackLogger.logFolderPath = dir + BStackLogger.logFilePath = path.join(dir, 'bstack-wdio-service.log') + }) + + afterEach(() => { + BStackLogger.clearLogger() + BStackLogger.logFilePath = originalPath + BStackLogger.logFolderPath = originalFolder + fs.rmSync(dir, { recursive: true, force: true }) + }) + + it('puts a just-logged line on disk, which an unflushed write does not', () => { + BStackLogger.debug('LINE_BEFORE_FLUSH') + // Baseline: without the flush the stream has not even opened the file yet, so anything + // that snapshots the log at this instant ships a truncated copy (or none at all). + const beforeFlush = fs.existsSync(BStackLogger.logFilePath) + ? fs.readFileSync(BStackLogger.logFilePath, 'utf8') + : '' + expect(beforeFlush).not.toContain('LINE_BEFORE_FLUSH') + }) + + it('drains the buffer without ending the stream', async () => { + BStackLogger.debug('FIRST_LINE') + await BStackLogger.flushLogFile() + expect(fs.readFileSync(BStackLogger.logFilePath, 'utf8')).toContain('FIRST_LINE') + + // still writable afterwards -- unlike clearLogger(), which ends the stream + BStackLogger.debug('SECOND_LINE') + await BStackLogger.flushLogFile() + expect(fs.readFileSync(BStackLogger.logFilePath, 'utf8')).toContain('SECOND_LINE') + }) + + it('resolves without a stream open, and cannot hang the upload', async () => { + BStackLogger.clearLogger() + await expect(BStackLogger.flushLogFile()).resolves.toBeUndefined() + + // a stream that never invokes the write callback must still resolve, via the timeout + BStackLogger.debug('x') + const stuck = { writable: true, write: vi.fn(), end: vi.fn() } as unknown as typeof BStackLogger['logFileStream'] + // @ts-expect-error -- reaching into the private static is the point of the test + BStackLogger.logFileStream = stuck + await expect(BStackLogger.flushLogFile(50)).resolves.toBeUndefined() + // @ts-expect-error -- drop the fake so afterEach does not call end() on it + BStackLogger.logFileStream = null + }) +})