mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-21 12:20:48 +02:00
fix: harden browser provider activation
This commit is contained in:
@@ -382,6 +382,7 @@ export class BrowserManager {
|
||||
// BROWSE_EXTENSIONS_DIR points to an unpacked Chrome extension directory.
|
||||
// Extensions only work in headed mode, so we use an off-screen window.
|
||||
const extensionsDir = process.env.BROWSE_EXTENSIONS_DIR;
|
||||
if (extensionsDir) assertHeadedBrowserProvider();
|
||||
const { STEALTH_LAUNCH_ARGS, buildGStackLaunchArgs } = await import('./stealth');
|
||||
const launchArgs: string[] = [...STEALTH_LAUNCH_ARGS, ...buildGStackLaunchArgs()];
|
||||
let useHeadless = true;
|
||||
@@ -1587,6 +1588,7 @@ export class BrowserManager {
|
||||
* If step 2 fails → return error, headless browser untouched
|
||||
*/
|
||||
async handoff(message: string): Promise<string> {
|
||||
assertHeadedBrowserProvider();
|
||||
if (this.connectionMode === 'headed' || this.isHeaded) {
|
||||
return `HANDOFF: Already in headed mode at ${this.getCurrentUrl()}`;
|
||||
}
|
||||
|
||||
+10
-5
@@ -118,7 +118,7 @@ interface ServerState {
|
||||
serverPath: string;
|
||||
binaryVersion?: string;
|
||||
mode?: 'launched' | 'headed';
|
||||
/** Hash of (proxyUrl + headed flag), used by D2 daemon-mismatch check. */
|
||||
/** Hash of proxy, headed mode, and browser-provider intent, used by daemon-mismatch checks. */
|
||||
configHash?: string;
|
||||
/** Xvfb child PID for cleanup on disconnect. */
|
||||
xvfbPid?: number;
|
||||
@@ -431,8 +431,8 @@ async function ensureServer(flags?: GlobalFlags): Promise<ServerState> {
|
||||
// hint. No silent restart — that would drop tab state, cookies, and
|
||||
// logged-in sessions without warning.
|
||||
if (desiredHash && state.configHash && state.configHash !== desiredHash) {
|
||||
console.error(`[browse] existing daemon has different config (proxy/headed mismatch).`);
|
||||
console.error(`[browse] run 'browse disconnect' first to apply --proxy/--headed.`);
|
||||
console.error(`[browse] existing daemon has different config (browser provider, proxy, or headed mode).`);
|
||||
console.error(`[browse] run 'browse disconnect' first to apply the selected browser configuration.`);
|
||||
process.exit(1);
|
||||
}
|
||||
// Same path: existing daemon is plain (no flags) but caller passes
|
||||
@@ -782,7 +782,7 @@ export interface GlobalFlags {
|
||||
proxyUrl: string | null;
|
||||
/** Whether --headed was passed. */
|
||||
headed: boolean;
|
||||
/** Hash of (proxy + headed) for daemon-mismatch check. */
|
||||
/** Hash of proxy, headed mode, and browser-provider intent for daemon-mismatch checks. */
|
||||
configHash: string;
|
||||
/** Redacted form of proxyUrl, safe for logs. */
|
||||
redactedProxyUrl: string;
|
||||
@@ -842,7 +842,12 @@ export function extractGlobalFlags(rawArgs: string[], env: NodeJS.ProcessEnv): G
|
||||
args: out,
|
||||
proxyUrl: canonicalProxyUrl,
|
||||
headed,
|
||||
configHash: computeConfigHash({ proxyUrl: canonicalProxyUrl, headed }),
|
||||
configHash: computeConfigHash({
|
||||
proxyUrl: canonicalProxyUrl,
|
||||
headed,
|
||||
browserProvider: env.GSTACK_BROWSER_PROVIDER,
|
||||
browserExecutable: env.GSTACK_CHROMIUM_PATH,
|
||||
}),
|
||||
redactedProxyUrl: redactProxyUrl(canonicalProxyUrl),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -125,7 +125,7 @@ export function toUpstreamConfig(cfg: ParsedProxyConfig): UpstreamConfig {
|
||||
}
|
||||
|
||||
/**
|
||||
* Compute a stable hash of (proxyUrl + headed flag) for daemon-mismatch
|
||||
* Compute a stable hash of proxy, headed mode, and browser-provider intent for daemon-mismatch
|
||||
* detection (D2). The hash is deterministic across CLI invocations on the
|
||||
* same machine and survives daemon restarts via the state file.
|
||||
*
|
||||
@@ -135,9 +135,18 @@ export function toUpstreamConfig(cfg: ParsedProxyConfig): UpstreamConfig {
|
||||
export function computeConfigHash(opts: {
|
||||
proxyUrl: string | null | undefined;
|
||||
headed: boolean;
|
||||
browserProvider?: string | null;
|
||||
browserExecutable?: string | null;
|
||||
}): string {
|
||||
const proxyKey = canonicalizeProxyUrl(opts.proxyUrl);
|
||||
const input = JSON.stringify({ proxy: proxyKey, headed: opts.headed });
|
||||
const browserProvider = opts.browserProvider || null;
|
||||
const browserExecutable = browserProvider === "installed" ? opts.browserExecutable || null : null;
|
||||
const input = JSON.stringify({
|
||||
proxy: proxyKey,
|
||||
headed: opts.headed,
|
||||
browserProvider,
|
||||
browserExecutable,
|
||||
});
|
||||
return createHash('sha256').update(input).digest('hex').slice(0, 16);
|
||||
}
|
||||
|
||||
|
||||
@@ -355,11 +355,18 @@ export async function handleWriteCommand(
|
||||
}
|
||||
} catch (err: any) {
|
||||
// Enhanced error guidance: clicking <option> elements always fails (not visible / timeout)
|
||||
const isOption = 'locator' in resolved
|
||||
? await resolved.locator.evaluate(el => el.tagName === 'OPTION').catch(() => false)
|
||||
: await target.locator(resolved.selector).evaluate(
|
||||
el => el.tagName === 'OPTION'
|
||||
).catch(() => false);
|
||||
// Do not start a second auto-wait after the click has already timed out.
|
||||
// Missing selectors used to spend 5s in click(), then block again in
|
||||
// evaluate() until the outer client killed the command. count() is an
|
||||
// immediate query and keeps the helpful option guidance only when one
|
||||
// unique element actually exists.
|
||||
const optionLocator = 'locator' in resolved
|
||||
? resolved.locator
|
||||
: target.locator(resolved.selector);
|
||||
const optionCount = await optionLocator.count().catch(() => 0);
|
||||
const isOption = optionCount === 1
|
||||
? await optionLocator.evaluate(el => el.tagName === 'OPTION').catch(() => false)
|
||||
: false;
|
||||
if (isOption) {
|
||||
throw new Error(
|
||||
`Cannot click <option> elements. Use 'browse select <parent-select> <value>' instead of 'click' for dropdown options.`
|
||||
|
||||
@@ -478,6 +478,20 @@ describe('Interaction', () => {
|
||||
}
|
||||
}, 15000);
|
||||
|
||||
test('click on a missing selector does not start a second locator wait', async () => {
|
||||
await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm);
|
||||
const started = performance.now();
|
||||
try {
|
||||
await handleWriteCommand('click', ['#definitely-missing-regression-node'], bm);
|
||||
expect(true).toBe(false); // Should not reach here
|
||||
} catch (err: any) {
|
||||
expect(err.message).toContain('#definitely-missing-regression-node');
|
||||
}
|
||||
// click() intentionally retains Playwright's 5s auto-wait. The regression
|
||||
// was a second default locator wait that pushed the total beyond 8s.
|
||||
expect(performance.now() - started).toBeLessThan(6500);
|
||||
}, 8000);
|
||||
|
||||
test('hover works', async () => {
|
||||
const result = await handleWriteCommand('hover', ['h1'], bm);
|
||||
expect(result).toContain('Hovered');
|
||||
|
||||
@@ -92,6 +92,47 @@ describe('D2 daemon-mismatch refuse (CLI integration)', () => {
|
||||
}
|
||||
}, 15000);
|
||||
|
||||
test('refuses to reuse a same-version daemon from a different browser provider', async () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-provider-mismatch-'));
|
||||
const stateFile = path.join(tmpDir, 'browse.json');
|
||||
const fakeServer = await startFakeHealthServer('fake-token');
|
||||
const { computeConfigHash } = await import('../src/proxy-config');
|
||||
const managedHash = computeConfigHash({
|
||||
proxyUrl: null,
|
||||
headed: false,
|
||||
browserProvider: 'managed',
|
||||
});
|
||||
|
||||
fs.writeFileSync(stateFile, JSON.stringify({
|
||||
pid: process.pid,
|
||||
port: fakeServer.port,
|
||||
token: 'fake-token',
|
||||
startedAt: new Date().toISOString(),
|
||||
serverPath: '',
|
||||
mode: 'launched',
|
||||
configHash: managedHash,
|
||||
}, null, 2));
|
||||
|
||||
const cliEnv: Record<string, string> = {};
|
||||
for (const [key, value] of Object.entries(process.env)) {
|
||||
if (value !== undefined) cliEnv[key] = value;
|
||||
}
|
||||
cliEnv.BROWSE_STATE_FILE = stateFile;
|
||||
cliEnv.GSTACK_BROWSER_PROVIDER = 'installed';
|
||||
cliEnv.GSTACK_CHROMIUM_PATH = process.execPath;
|
||||
|
||||
try {
|
||||
const result = await runCli(['status'], cliEnv);
|
||||
expect(result.code).toBe(1);
|
||||
expect(result.stderr).toContain('different config');
|
||||
expect(result.stderr).toContain('browse disconnect');
|
||||
} finally {
|
||||
await fakeServer.close();
|
||||
try { fs.unlinkSync(stateFile); } catch { /* ignore */ }
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
}, 15000);
|
||||
|
||||
test('refuses when existing plain daemon meets a --proxy invocation', async () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-mismatch-plain-'));
|
||||
const stateFile = path.join(tmpDir, 'browse.json');
|
||||
|
||||
@@ -186,4 +186,20 @@ describe('extractGlobalFlags', () => {
|
||||
);
|
||||
expect(a.configHash).not.toBe(b.configHash);
|
||||
});
|
||||
|
||||
test('configHash changes with browser provider and installed executable', () => {
|
||||
const managed = extractGlobalFlags(['goto', 'x'], {
|
||||
GSTACK_BROWSER_PROVIDER: 'managed',
|
||||
} as NodeJS.ProcessEnv);
|
||||
const installedA = extractGlobalFlags(['goto', 'x'], {
|
||||
GSTACK_BROWSER_PROVIDER: 'installed',
|
||||
GSTACK_CHROMIUM_PATH: '/browser/a',
|
||||
} as NodeJS.ProcessEnv);
|
||||
const installedB = extractGlobalFlags(['goto', 'x'], {
|
||||
GSTACK_BROWSER_PROVIDER: 'installed',
|
||||
GSTACK_CHROMIUM_PATH: '/browser/b',
|
||||
} as NodeJS.ProcessEnv);
|
||||
expect(managed.configHash).not.toBe(installedA.configHash);
|
||||
expect(installedA.configHash).not.toBe(installedB.configHash);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user