Skip to content

Commit 766d67e

Browse files
committed
fix(FR-2877): drop @smoke-any from grep, clarify TLS env mutation, simplify argv scrub
1 parent 6590cb4 commit 766d67e

5 files changed

Lines changed: 173 additions & 50 deletions

File tree

packages/backend.ai-webui-smoke-cli/README.md

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,51 @@ for the full spec. The operator-facing README (EN + KO) lands in FR-2883
77

88
## Usage (alpha)
99

10+
Prefer reading the password from stdin or an env var — passing
11+
`--password` on the command line exposes the secret to anything that
12+
can read the host process's argv. The CLI scrubs the argv value at
13+
startup, but the original invocation may still surface in shell history
14+
or process listings before the scrub runs.
15+
1016
```sh
11-
bai-smoke run \
17+
# Option 1 — password from stdin (recommended)
18+
printf '%s' "$BAI_PASSWORD" | bai-smoke run \
19+
--endpoint https://webui.example.com \
20+
--webserver https://webui.example.com \
21+
--email admin@example.com \
22+
--password-stdin \
23+
--output ./smoke-report
24+
25+
# Option 2 — password from env var
26+
BAI_SMOKE_PASSWORD="$BAI_PASSWORD" bai-smoke run \
1227
--endpoint https://webui.example.com \
28+
--webserver https://webui.example.com \
1329
--email admin@example.com \
14-
--password "***" \
1530
--output ./smoke-report
1631
```
1732

18-
This is an alpha MVP. Full operator docs land with FR-2883.
33+
To widen the selection beyond the default `@smoke*` set, use
34+
`--also-include`. To narrow the run, use `--pages`:
35+
36+
```sh
37+
bai-smoke run \
38+
--endpoint https://webui.example.com \
39+
--email admin@example.com \
40+
--password-stdin \
41+
--also-include "@critical" \
42+
--pages session,vfolder
43+
```
44+
45+
## Limitations (alpha MVP)
46+
47+
- Must be run from a `backend.ai-webui` monorepo checkout — the e2e
48+
specs are not bundled yet. Tarball / single-binary distribution is
49+
tracked in FR-2881.
50+
- Air-gap binary, `doctor` command, and rich diagnostic reports → Phase 2.
51+
- `--also-include` widens the smoke set; to narrow it, use `--pages`
52+
instead. The legacy `--include` flag is a deprecated hidden alias and
53+
will be removed in a future release.
54+
- `--insecure-tls` accepts self-signed certs but does not pin
55+
fingerprints — only enable on trusted networks.
56+
57+
This is an alpha MVP. Full operator docs (EN + KO) land with FR-2883.

packages/backend.ai-webui-smoke-cli/playwright.smoke.config.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
import { defineConfig, devices } from '@playwright/test';
2+
import { existsSync } from 'node:fs';
23
import path from 'node:path';
34
import { fileURLToPath } from 'node:url';
45

56
/**
67
* Playwright configuration for the `bai-smoke` runner.
78
*
8-
* This config is invoked by `bai-smoke run` (`src/runner.ts`) via
9-
* `npx playwright test --config <this-file>`. The runner is the single
9+
* This config is invoked by `bai-smoke run` (`src/runner.ts`), which
10+
* spawns Node directly with the resolved `@playwright/test` CLI
11+
* entrypoint and passes `--config <this-file>`. No `npx` is involved.
12+
* The runner is the single
1013
* place that translates CLI arguments into the env vars consumed here —
1114
* we deliberately do NOT load `e2e/envs/.env.playwright` from this file
1215
* because the smoke CLI runs against arbitrary customer endpoints, not
@@ -16,7 +19,7 @@ import { fileURLToPath } from 'node:url';
1619
* BAI_SMOKE_REPORT_DIR — output directory for html / json reports
1720
* BAI_SMOKE_WORKERS — playwright worker count (optional)
1821
* BAI_SMOKE_TIMEOUT_MS — per-test timeout in ms (default 180000)
19-
* BAI_SMOKE_GREP — grep regex source (no slashes), e.g. `@smoke(-any|-admin)?\b`
22+
* BAI_SMOKE_GREP — grep regex source (no slashes), e.g. `@smoke\b|@smoke-admin\b`
2023
* BAI_SMOKE_GREP_INVERT — grepInvert regex source (optional)
2124
* BAI_SMOKE_PAGES — comma-separated page directory names (optional)
2225
* BAI_SMOKE_HEADED — '1' to launch a headed browser
@@ -31,6 +34,18 @@ const __filename = fileURLToPath(import.meta.url);
3134
const __dirname = path.dirname(__filename);
3235
const E2E_DIR = path.resolve(__dirname, '..', '..', 'e2e');
3336

37+
// FR-2877 MVP limitation: the smoke runner discovers specs from the
38+
// monorepo's e2e/ tree. A bundled tarball / standalone binary distribution
39+
// is tracked in FR-2881. Fail loudly when the directory is missing so the
40+
// user gets a clear message instead of a cryptic Playwright "no tests
41+
// found" output.
42+
if (!existsSync(E2E_DIR)) {
43+
throw new Error(
44+
`bai-smoke MVP requires running from a backend.ai-webui checkout. Expected e2e dir at: ${E2E_DIR}. ` +
45+
`Tarball / single-binary distribution lands in FR-2881.`,
46+
);
47+
}
48+
3449
const reportDir = process.env.BAI_SMOKE_REPORT_DIR ?? path.resolve(process.cwd(), 'smoke-report');
3550

3651
const workersEnv = process.env.BAI_SMOKE_WORKERS;
@@ -75,7 +90,7 @@ export default defineConfig({
7590
trace: 'retain-on-failure',
7691
video: 'retain-on-failure',
7792
headless: !headed,
78-
ignoreHTTPSErrors: true,
93+
ignoreHTTPSErrors: process.env.BAI_SMOKE_INSECURE_TLS === '1',
7994
},
8095
projects: [
8196
{

packages/backend.ai-webui-smoke-cli/src/cli.ts

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,16 @@ program
9696
.choices(['auto', 'admin', 'user'])
9797
.default('auto'),
9898
)
99-
.option('--include <tags>', 'Comma-separated extra tags to include (e.g. "@critical").')
99+
.option(
100+
'--also-include <tags>',
101+
'Additional tag pattern(s) to OR onto the smoke selection (e.g. "@critical"). ' +
102+
'To NARROW the smoke set, use --pages instead.',
103+
)
104+
// Backwards-compat alias for the original `--include` name. Deprecated —
105+
// will be removed in a future release. Kept hidden from the main help.
106+
.addOption(
107+
new Option('--include <tags>', '(deprecated) alias for --also-include.').hideHelp(),
108+
)
100109
.option('--exclude <tags>', 'Comma-separated tags to exclude.')
101110
.option(
102111
'--pages <names>',
@@ -108,12 +117,7 @@ program
108117
'Per-test timeout. Accepts "180s", "3m", or raw ms.',
109118
'180s',
110119
)
111-
.option(
112-
'--output <dir>',
113-
'Output directory for the smoke report.',
114-
() => defaultOutputDir(),
115-
defaultOutputDir(),
116-
)
120+
.option('--output <dir>', 'Output directory for the smoke report.')
117121
.option('--headed', 'Run the browser in headed mode (debugging only).', false)
118122
.option('--insecure-tls', 'Accept self-signed TLS certificates.', false)
119123
.action(async (raw: Record<string, unknown>) => {
@@ -124,6 +128,9 @@ program
124128
);
125129
process.exit(2);
126130
}
131+
// Scrub the password value from process.argv so accidental introspection
132+
// (logs, crash dumps, `ps`-style helpers reading argv) cannot leak it.
133+
scrubPasswordFromArgv();
127134

128135
const endpoint = String(raw.endpoint);
129136
const webserver =
@@ -145,18 +152,25 @@ program
145152
return;
146153
}
147154

155+
// `--also-include` is the canonical flag; `--include` is a hidden alias.
156+
const alsoIncludeRaw =
157+
(raw.alsoInclude as string | string[] | undefined) ??
158+
(raw.include as string | string[] | undefined);
159+
160+
const outputDir = path.resolve(String(raw.output ?? defaultOutputDir()));
161+
148162
const opts: SmokeRunOptions = {
149163
endpoint,
150164
webserver,
151165
email: String(raw.email),
152166
password,
153167
role: (raw.role as SmokeRoleSelection) ?? 'auto',
154-
include: splitCsvArg(raw.include as string | string[] | undefined),
168+
include: splitCsvArg(alsoIncludeRaw),
155169
exclude: splitCsvArg(raw.exclude as string | string[] | undefined),
156170
pages: splitCsvArg(raw.pages as string | string[] | undefined),
157171
workers: typeof raw.workers === 'number' && raw.workers > 0 ? raw.workers : undefined,
158172
timeoutMs,
159-
outputDir: path.resolve(String(raw.output ?? defaultOutputDir())),
173+
outputDir,
160174
headed: raw.headed === true,
161175
insecureTls: raw.insecureTls === true,
162176
};
@@ -203,6 +217,27 @@ async function resolvePassword(raw: Record<string, unknown>): Promise<string | u
203217
return undefined;
204218
}
205219

220+
/**
221+
* Replace the resolved password value in `process.argv` with `***` so
222+
* downstream introspection (crash dumps, logs that dump argv, child
223+
* processes inheriting argv via APIs that read the parent's command
224+
* line) cannot leak the secret. Covers both `--password VALUE` and
225+
* `--password=VALUE` forms.
226+
*/
227+
function scrubPasswordFromArgv(): void {
228+
for (let i = 0; i < process.argv.length; i++) {
229+
const a = process.argv[i];
230+
if (a == null) continue;
231+
if (a.startsWith('--password=')) {
232+
process.argv[i] = '--password=***';
233+
continue;
234+
}
235+
if (a === '--password' && process.argv[i + 1] != null) {
236+
process.argv[i + 1] = '***';
237+
}
238+
}
239+
}
240+
206241
function readStdin(): Promise<string> {
207242
return new Promise((resolve, reject) => {
208243
const chunks: Buffer[] = [];

packages/backend.ai-webui-smoke-cli/src/config.ts

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@ export interface SmokeRunOptions {
2020
password: string;
2121
/** Role selection mode (`--role`). */
2222
role: SmokeRoleSelection;
23-
/** Extra include tags (`--include @critical,@dashboard` → `['@critical','@dashboard']`). */
23+
/**
24+
* Extra tags to OR onto the smoke selection. Populated from
25+
* `--also-include` (preferred) or the deprecated `--include` alias.
26+
*/
2427
include?: string[];
2528
/** Exclude tags (`--exclude`). */
2629
exclude?: string[];
@@ -42,27 +45,31 @@ export interface SmokeRunOptions {
4245
* Build the grep / grepInvert regex pair forwarded to Playwright via env.
4346
*
4447
* The base smoke set is `@smoke`; on top of that, each test may carry
45-
* a role-scoped tag (`@smoke-admin`, `@smoke-user`) or the role-agnostic
46-
* `@smoke-any`. We compose a regex that accepts:
48+
* a role-scoped tag (`@smoke-admin`, `@smoke-user`). We compose a regex
49+
* that accepts:
4750
*
48-
* - bare `@smoke` (with no -admin/-user/-any suffix), OR
49-
* - `@smoke-any`, OR
51+
* - bare `@smoke` (with no role suffix), OR
5052
* - `@smoke-<effectiveRole>`
5153
*
52-
* so a `user` run picks up `@smoke`, `@smoke-any`, and `@smoke-user` —
53-
* but NOT `@smoke-admin`.
54+
* so a `user` run picks up `@smoke` and `@smoke-user` — but NOT
55+
* `@smoke-admin`.
5456
*
55-
* Additional `--include` tags are OR-ed in. `--exclude` tags become
57+
* (`@smoke-any` was dropped from the taxonomy in FR-2875 / FR-2876
58+
* because every e2e helper hard-codes a role via `loginAsAdmin` /
59+
* `loginAsUser`, so no describe is genuinely role-agnostic at the
60+
* helper level.)
61+
*
62+
* Additional `--also-include` tags are OR-ed in. `--exclude` tags become
5663
* `grepInvert`.
5764
*/
5865
export function buildGrepExpression(
5966
opts: SmokeRunOptions,
6067
effectiveRole: EffectiveRole,
6168
): { grep?: string; grepInvert?: string } {
62-
// `@smoke\b` matches the bare base tag and we cover `@smoke-any` /
63-
// `@smoke-<role>` explicitly so we don't accidentally pick up
64-
// future tags like `@smoke-extended`.
65-
const roleAlt = `@smoke-any\\b|@smoke-${effectiveRole}\\b`;
69+
// `@smoke\b` matches the bare base tag; we match `@smoke-<role>`
70+
// explicitly so we don't accidentally pick up future tags like
71+
// `@smoke-extended`.
72+
const roleAlt = `@smoke-${effectiveRole}\\b`;
6673
const baseAlt = `@smoke\\b`;
6774
const includeAlt = (opts.include ?? [])
6875
.map((t) => escapeRegex(t.trim()))
@@ -122,8 +129,11 @@ export function buildPlaywrightEnv(
122129

123130
if (opts.insecureTls) {
124131
// Forward to both the runner's spawned children and any fetches
125-
// inside the test harness. Only set when explicitly requested.
132+
// inside the test harness. Only set on the spawned child env — we
133+
// intentionally do NOT mutate process.env in the host runner.
126134
env.NODE_TLS_REJECT_UNAUTHORIZED = '0';
135+
// Gate Playwright's `ignoreHTTPSErrors` in playwright.smoke.config.ts.
136+
env.BAI_SMOKE_INSECURE_TLS = '1';
127137
}
128138

129139
return env;

packages/backend.ai-webui-smoke-cli/src/runner.ts

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ export interface SmokeSummary {
3232
webserver: string;
3333
role: EffectiveRole;
3434
roleSelection: SmokeRunOptions['role'];
35-
include?: string[];
35+
/** Tags OR-ed onto the smoke selection via `--also-include`. */
36+
alsoInclude?: string[];
3637
exclude?: string[];
3738
pages?: string[];
3839
grep?: string;
@@ -56,27 +57,54 @@ export interface SmokeSummary {
5657
/**
5758
* Auto-detect role by hitting the webserver's /server/login endpoint.
5859
*
59-
* On failure we conservatively default to `'user'` — fewer privileged
60-
* specs attempted is safer than running admin-only specs against a
61-
* misclassified account.
60+
* On any failure (network error, non-200 status, malformed JSON,
61+
* `authenticated !== true`) we throw. Silently defaulting to `'user'`
62+
* was masking real misconfiguration — operators ended up with a green
63+
* smoke run that had skipped every admin-tagged spec without saying so.
64+
*
65+
* The body shape we POST here (`{ username, password }`) mirrors what
66+
* the SESSION-mode browser fixture submits to `/server/login`. The
67+
* proper signed-request based detection — using the post-login
68+
* `/func/auth/role` endpoint — is tracked in TODO(FR-2878).
6269
*/
6370
export async function detectRole(opts: SmokeRunOptions): Promise<EffectiveRole> {
6471
const url = `${opts.webserver.replace(/\/$/, '')}/server/login`;
6572
// Honour --insecure-tls for the detection call so self-signed certs
6673
// don't crash detection before we even reach Playwright.
74+
//
75+
// Note: this temporarily mutates the *current* (host) process env —
76+
// `NODE_TLS_REJECT_UNAUTHORIZED` is a Node-wide knob and there is no
77+
// per-call switch for the built-in `fetch`. We restore the previous
78+
// value (or delete the key if it was unset) in `finally`, so the
79+
// mutation is scoped to the duration of this detection request.
80+
// The same flag is also forwarded to the spawned Playwright process
81+
// via `buildPlaywrightEnv` so the in-browser HTTPS calls inherit it.
6782
const previousTlsReject = process.env.NODE_TLS_REJECT_UNAUTHORIZED;
6883
if (opts.insecureTls) process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';
6984
try {
70-
const res = await fetch(url, {
71-
method: 'POST',
72-
headers: { 'Content-Type': 'application/json' },
73-
body: JSON.stringify({ username: opts.email, password: opts.password }),
74-
});
85+
let res: Response;
86+
try {
87+
res = await fetch(url, {
88+
method: 'POST',
89+
headers: { 'Content-Type': 'application/json' },
90+
// TODO(FR-2878): switch to signed-request /func/auth/role for a
91+
// proper detection. This naive form matches what the SESSION-mode
92+
// browser fixture submits to /server/login and works against the
93+
// current staging cluster.
94+
body: JSON.stringify({ username: opts.email, password: opts.password }),
95+
});
96+
} catch (err) {
97+
throw new Error(
98+
`Failed to auto-detect role for ${opts.email} against ${opts.webserver}: ` +
99+
`${(err as Error).message}. Pass --role admin or --role user explicitly and retry.`,
100+
);
101+
}
75102
if (!res.ok) {
76-
process.stderr.write(
77-
`[bai-smoke] role detection: webserver returned HTTP ${res.status}; defaulting to "user".\n`,
103+
throw new Error(
104+
`Failed to auto-detect role for ${opts.email} against ${opts.webserver}: ` +
105+
`webserver returned HTTP ${res.status}. ` +
106+
`Pass --role admin or --role user explicitly and retry.`,
78107
);
79-
return 'user';
80108
}
81109
const json = (await res.json().catch(() => null)) as
82110
| {
@@ -85,22 +113,18 @@ export async function detectRole(opts: SmokeRunOptions): Promise<EffectiveRole>
85113
}
86114
| null;
87115
if (!json || json.authenticated !== true) {
88-
process.stderr.write(
89-
'[bai-smoke] role detection: login not authenticated; defaulting to "user".\n',
116+
throw new Error(
117+
`Failed to auto-detect role for ${opts.email} against ${opts.webserver}: ` +
118+
`login not authenticated (unexpected response shape or wrong credentials). ` +
119+
`Pass --role admin or --role user explicitly and retry.`,
90120
);
91-
return 'user';
92121
}
93122
const role = json.data?.role;
94123
const isAdmin = json.data?.is_admin === true;
95124
if (role === 'admin' || role === 'superadmin' || isAdmin) {
96125
return 'admin';
97126
}
98127
return 'user';
99-
} catch (err) {
100-
process.stderr.write(
101-
`[bai-smoke] role detection failed: ${(err as Error).message}; defaulting to "user".\n`,
102-
);
103-
return 'user';
104128
} finally {
105129
if (opts.insecureTls) {
106130
if (previousTlsReject === undefined) {
@@ -175,7 +199,7 @@ export async function runSmoke(opts: SmokeRunOptions): Promise<SmokeRunResult> {
175199
webserver: opts.webserver,
176200
role: effectiveRole,
177201
roleSelection: opts.role,
178-
include: opts.include,
202+
alsoInclude: opts.include,
179203
exclude: opts.exclude,
180204
pages: opts.pages,
181205
grep,

0 commit comments

Comments
 (0)