Skip to content

Commit 380e11b

Browse files
committed
fix(daemon): enforce provider session artifact ownership
1 parent 3b468c0 commit 380e11b

9 files changed

Lines changed: 525 additions & 34 deletions

File tree

packages/provider-webdriver/src/runtime-session.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,33 @@ test('a create-timeout surfaces provider evidence for the maybe-leaked session',
7272
}
7373
});
7474

75+
test('cloud artifact lookup does not accept a provider session id without its lease', async () => {
76+
let listCalls = 0;
77+
const runtime = makeRuntime({
78+
listArtifacts: async () => {
79+
listCalls += 1;
80+
return {
81+
provider: 'webdriver-test',
82+
status: 'ready',
83+
cloudArtifacts: [],
84+
};
85+
},
86+
});
87+
88+
try {
89+
const cloudArtifacts = runtime.cloudArtifacts;
90+
assert.ok(cloudArtifacts);
91+
const result = await cloudArtifacts.listCloudArtifacts?.({
92+
provider: 'webdriver-test',
93+
providerSessionId: 'never-authorized',
94+
});
95+
assert.equal(result, undefined);
96+
assert.equal(listCalls, 0);
97+
} finally {
98+
await runtime.shutdown();
99+
}
100+
});
101+
75102
function makeRuntime(overrides: Partial<CloudWebDriverRuntimeOptions> = {}) {
76103
return createCloudWebDriverRuntime({
77104
clientVersion: 'test',

packages/provider-webdriver/src/runtime-session.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -146,12 +146,25 @@ export class WebDriverSessionManager {
146146

147147
async listCloudArtifacts(query: CloudArtifactsQuery): Promise<CloudArtifactsResult | undefined> {
148148
if (query.provider !== this.options.provider) return undefined;
149-
const session = query.leaseId ? this.sessionsByLeaseId.get(query.leaseId) : undefined;
150-
if (session) return await this.safeListArtifacts(session);
151-
const providerSessionId =
152-
query.providerSessionId ??
153-
(query.leaseId ? this.releasedProviderSessionIdsByLeaseId.get(query.leaseId) : undefined);
149+
const session = this.sessionsByLeaseId.get(query.leaseId ?? '');
150+
if (session) return await this.listActiveCloudArtifacts(session, query.providerSessionId);
151+
return await this.listReleasedCloudArtifacts(query);
152+
}
153+
154+
private async listActiveCloudArtifacts(
155+
session: WebDriverProviderSession,
156+
providerSessionId: string | undefined,
157+
): Promise<CloudArtifactsResult | undefined> {
158+
if (providerSessionId && providerSessionId !== session.providerSessionId) return undefined;
159+
return await this.safeListArtifacts(session);
160+
}
161+
162+
private async listReleasedCloudArtifacts(
163+
query: CloudArtifactsQuery,
164+
): Promise<CloudArtifactsResult | undefined> {
165+
const providerSessionId = this.releasedProviderSessionIdsByLeaseId.get(query.leaseId ?? '');
154166
if (!providerSessionId || !this.options.listArtifacts) return undefined;
167+
if (query.providerSessionId && query.providerSessionId !== providerSessionId) return undefined;
155168
return await this.options.listArtifacts({
156169
provider: this.options.provider,
157170
providerSessionId,

src/__tests__/daemon-entrypoint.test.ts

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -170,11 +170,10 @@ test('daemon runtime publishes dual transport metadata', async () => {
170170
}
171171
});
172172

173-
test('daemon default provider composition serves cloud artifacts over RPC', async () => {
173+
test('daemon rejects unowned cloud artifacts over RPC', async () => {
174174
const stateDir = mkdtempForTestSync('agent-device-daemon-provider-');
175175
const providerRequests: string[] = [];
176-
const providerServer = http.createServer((req, res) => {
177-
providerRequests.push(req.url ?? '');
176+
const providerServer = http.createServer((_req, res) => {
178177
res.setHeader('content-type', 'application/json');
179178
res.end(
180179
JSON.stringify({
@@ -219,27 +218,14 @@ test('daemon default provider composition serves cloud artifacts over RPC', asyn
219218
}),
220219
});
221220
const body = (await response.json()) as {
222-
result?: { ok?: boolean; data?: Record<string, unknown> };
221+
error?: { code?: number; message?: string; data?: { code?: string; details?: unknown } };
223222
};
224223

225-
assert.equal(response.status, 200);
226-
assert.equal(body.result?.ok, true);
227-
assert.deepEqual(body.result?.data, {
228-
provider: 'browserstack',
229-
providerSessionId: 'wd-1',
230-
status: 'ready',
231-
cloudArtifacts: [
232-
{
233-
provider: 'browserstack',
234-
providerSessionId: 'wd-1',
235-
kind: 'video',
236-
name: 'Session video',
237-
url: 'https://browserstack.example/video.mp4',
238-
availability: 'ready',
239-
},
240-
],
241-
});
242-
assert.deepEqual(providerRequests, ['/sessions/wd-1.json']);
224+
assert.equal(response.status, 401);
225+
assert.equal(body.error?.code, -32000);
226+
assert.equal(body.error?.data?.code, 'UNAUTHORIZED');
227+
assert.deepEqual(body.error?.data?.details, { reason: 'PROVIDER_SESSION_NOT_OWNED' });
228+
assert.deepEqual(providerRequests, []);
243229
} finally {
244230
await runtime?.shutdown();
245231
await closeLoopbackServer(providerServer);

src/daemon/__tests__/lease-lifecycle.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,41 @@ test('releaseSessionLease releases with the stored session owner scope', async (
120120
expect(provider).toEqual({ provider: 'proxy' });
121121
});
122122

123+
test('releaseSessionLease retains provider session ownership for artifact lookup', async () => {
124+
const leaseRegistry = new LeaseRegistry();
125+
const lease = leaseRegistry.allocateLease({
126+
tenantId: 'tenant-a',
127+
runId: 'run-1',
128+
leaseBackend: 'android-instance',
129+
leaseProvider: 'browserstack',
130+
});
131+
const session = makeIosSession('default', {
132+
lease: {
133+
leaseId: lease.leaseId,
134+
tenantId: lease.tenantId,
135+
runId: lease.runId,
136+
leaseBackend: lease.backend,
137+
leaseProvider: lease.leaseProvider,
138+
},
139+
});
140+
141+
await releaseSessionLease({
142+
session,
143+
leaseRegistry,
144+
leaseLifecycleProvider: {
145+
release: async () => ({ providerSessionId: 'bs-session-1' }),
146+
},
147+
});
148+
149+
expect(
150+
leaseRegistry.resolveProviderSession({
151+
provider: 'browserstack',
152+
providerSessionId: 'bs-session-1',
153+
tenantId: 'tenant-a',
154+
}),
155+
).toMatchObject({ leaseId: lease.leaseId, tenantId: 'tenant-a' });
156+
});
157+
123158
test('releaseExpiredProviderLease releases a provider-owned lease without a session', async () => {
124159
const lease = new LeaseRegistry().allocateLease({
125160
tenantId: 'tenant-a',

0 commit comments

Comments
 (0)