Skip to content

Commit 926a905

Browse files
authored
fix(device-agent): unbrick stuck installs (auto-update + legacy session)
fix(device-agent): unbrick stuck installs (auto-update + legacy session)
1 parent d75a835 commit 926a905

6 files changed

Lines changed: 374 additions & 8 deletions

File tree

apps/api/src/device-agent/device-agent.controller.spec.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import { PermissionGuard } from '../auth/permission.guard';
88
import type { AuthContext as AuthContextType } from '../auth/types';
99
import { Readable } from 'stream';
1010

11+
jest.mock('@db', () => ({ db: {} }));
12+
1113
jest.mock('../auth/auth.server', () => ({
1214
auth: { api: { getSession: jest.fn() } },
1315
}));
@@ -26,6 +28,8 @@ describe('DeviceAgentController', () => {
2628
const mockService = {
2729
downloadMacAgent: jest.fn(),
2830
downloadWindowsAgent: jest.fn(),
31+
getUpdateFile: jest.fn(),
32+
headUpdateFile: jest.fn(),
2933
};
3034

3135
const mockGuard = { canActivate: jest.fn().mockReturnValue(true) };
@@ -42,6 +46,7 @@ describe('DeviceAgentController', () => {
4246

4347
const mockRes = {
4448
set: jest.fn(),
49+
redirect: jest.fn(),
4550
};
4651

4752
beforeEach(async () => {
@@ -155,4 +160,97 @@ describe('DeviceAgentController', () => {
155160
).rejects.toThrow('Agent not found');
156161
});
157162
});
163+
164+
describe('getUpdateFile', () => {
165+
it('streams a yml manifest with cache headers', async () => {
166+
const mockStream = Readable.from(Buffer.from('version: 1.0.5'));
167+
mockService.getUpdateFile.mockResolvedValue({
168+
kind: 'stream',
169+
stream: mockStream,
170+
contentType: 'text/yaml',
171+
contentLength: 14,
172+
});
173+
174+
const result = await controller.getUpdateFile(
175+
'latest-mac.yml',
176+
mockRes as never,
177+
);
178+
179+
expect(result).toBeInstanceOf(StreamableFile);
180+
expect(mockRes.set).toHaveBeenCalledWith(
181+
expect.objectContaining({
182+
'Content-Type': 'text/yaml',
183+
'Cache-Control': 'public, max-age=300',
184+
'Content-Length': '14',
185+
}),
186+
);
187+
expect(mockRes.redirect).not.toHaveBeenCalled();
188+
});
189+
190+
it('issues a 302 redirect to the presigned URL for binaries', async () => {
191+
mockService.getUpdateFile.mockResolvedValue({
192+
kind: 'redirect',
193+
url: 'https://s3.example.com/signed-zip',
194+
});
195+
196+
const result = await controller.getUpdateFile(
197+
'CompAI-Device-Agent-1.0.5-arm64.zip',
198+
mockRes as never,
199+
);
200+
201+
expect(mockRes.redirect).toHaveBeenCalledWith(
202+
302,
203+
'https://s3.example.com/signed-zip',
204+
);
205+
expect(mockRes.set).toHaveBeenCalledWith(
206+
expect.objectContaining({
207+
'Cache-Control': 'no-store',
208+
}),
209+
);
210+
expect(result).toBeUndefined();
211+
});
212+
});
213+
214+
describe('headUpdateFile', () => {
215+
it('returns metadata headers for a yml manifest', async () => {
216+
mockService.headUpdateFile.mockResolvedValue({
217+
kind: 'stream',
218+
contentType: 'text/yaml',
219+
contentLength: 859,
220+
});
221+
222+
const result = await controller.headUpdateFile(
223+
'latest-mac.yml',
224+
mockRes as never,
225+
);
226+
227+
expect(mockRes.set).toHaveBeenCalledWith(
228+
expect.objectContaining({
229+
'Content-Type': 'text/yaml',
230+
'Cache-Control': 'public, max-age=300',
231+
'Content-Length': '859',
232+
}),
233+
);
234+
expect(result).toBe('');
235+
expect(mockRes.redirect).not.toHaveBeenCalled();
236+
});
237+
238+
it('issues a 302 redirect for HEAD on binaries', async () => {
239+
mockService.headUpdateFile.mockResolvedValue({
240+
kind: 'redirect',
241+
url: 'https://s3.example.com/signed-head',
242+
});
243+
244+
const result = await controller.headUpdateFile(
245+
'CompAI-Device-Agent-1.0.5-arm64.zip',
246+
mockRes as never,
247+
);
248+
249+
expect(mockRes.redirect).toHaveBeenCalledWith(
250+
302,
251+
'https://s3.example.com/signed-head',
252+
);
253+
expect(result).toBeUndefined();
254+
});
255+
});
158256
});

apps/api/src/device-agent/device-agent.controller.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,12 @@ export class DeviceAgentController {
7070
) {
7171
const result = await this.deviceAgentService.getUpdateFile({ filename });
7272

73+
if (result.kind === 'redirect') {
74+
res.set({ 'Cache-Control': 'no-store' });
75+
res.redirect(302, result.url);
76+
return;
77+
}
78+
7379
res.set({
7480
'Content-Type': result.contentType,
7581
'Cache-Control': 'public, max-age=300',
@@ -90,6 +96,12 @@ export class DeviceAgentController {
9096
) {
9197
const result = await this.deviceAgentService.headUpdateFile({ filename });
9298

99+
if (result.kind === 'redirect') {
100+
res.set({ 'Cache-Control': 'no-store' });
101+
res.redirect(302, result.url);
102+
return;
103+
}
104+
93105
res.set({
94106
'Content-Type': result.contentType,
95107
'Cache-Control': 'public, max-age=300',

apps/api/src/device-agent/device-agent.service.spec.ts

Lines changed: 136 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,28 @@ import {
55
import { Readable } from 'stream';
66

77
const mockSend = jest.fn();
8+
const mockGetSignedUrl = jest.fn();
9+
10+
class MockGetObjectCommand {
11+
constructor(public readonly input: unknown) {
12+
Object.assign(this, input as object);
13+
}
14+
}
15+
16+
class MockHeadObjectCommand {
17+
constructor(public readonly input: unknown) {
18+
Object.assign(this, input as object);
19+
}
20+
}
821

922
jest.mock('@aws-sdk/client-s3', () => ({
1023
S3Client: jest.fn().mockImplementation(() => ({ send: mockSend })),
11-
GetObjectCommand: jest.fn().mockImplementation((input) => input),
24+
GetObjectCommand: MockGetObjectCommand,
25+
HeadObjectCommand: MockHeadObjectCommand,
26+
}));
27+
28+
jest.mock('@aws-sdk/s3-request-presigner', () => ({
29+
getSignedUrl: (...args: unknown[]) => mockGetSignedUrl(...args),
1230
}));
1331

1432
import { DeviceAgentService } from './device-agent.service';
@@ -92,6 +110,123 @@ describe('DeviceAgentService', () => {
92110
});
93111
});
94112

113+
describe('getUpdateFile', () => {
114+
it('streams .yml manifests directly from S3', async () => {
115+
const mockStream = new Readable({ read() {} });
116+
mockSend.mockResolvedValue({
117+
Body: mockStream,
118+
ContentLength: 859,
119+
});
120+
121+
const result = await service.getUpdateFile({ filename: 'latest-mac.yml' });
122+
123+
expect(result).toEqual({
124+
kind: 'stream',
125+
stream: mockStream,
126+
contentType: 'text/yaml',
127+
contentLength: 859,
128+
});
129+
expect(mockGetSignedUrl).not.toHaveBeenCalled();
130+
});
131+
132+
it('redirects binary downloads to a presigned S3 URL signed for GET', async () => {
133+
mockGetSignedUrl.mockResolvedValue('https://s3.example.com/signed-zip-url');
134+
135+
const result = await service.getUpdateFile({
136+
filename: 'CompAI-Device-Agent-1.0.5-arm64.zip',
137+
});
138+
139+
expect(result).toEqual({
140+
kind: 'redirect',
141+
url: 'https://s3.example.com/signed-zip-url',
142+
});
143+
expect(mockSend).not.toHaveBeenCalled();
144+
expect(mockGetSignedUrl).toHaveBeenCalledTimes(1);
145+
const [, command] = mockGetSignedUrl.mock.calls[0];
146+
expect(command).toBeInstanceOf(MockGetObjectCommand);
147+
expect(command).toMatchObject({
148+
Bucket: 'test-bucket',
149+
Key: 'device-agent/production/updates/CompAI-Device-Agent-1.0.5-arm64.zip',
150+
});
151+
});
152+
153+
it.each([
154+
'CompAI-Device-Agent-1.0.5-arm64.zip',
155+
'CompAI-Device-Agent-1.0.5-setup.exe',
156+
'CompAI-Device-Agent-1.0.5-arm64.dmg',
157+
'CompAI-Device-Agent-1.0.5-x86_64.AppImage',
158+
'CompAI-Device-Agent-1.0.5-arm64.zip.blockmap',
159+
])('redirects binary file %s', async (filename) => {
160+
mockGetSignedUrl.mockResolvedValue('https://s3.example.com/signed');
161+
162+
const result = await service.getUpdateFile({ filename });
163+
164+
expect(result).toEqual({
165+
kind: 'redirect',
166+
url: 'https://s3.example.com/signed',
167+
});
168+
});
169+
170+
it('throws NotFoundException for invalid filenames', async () => {
171+
await expect(
172+
service.getUpdateFile({ filename: '../etc/passwd' }),
173+
).rejects.toThrow(NotFoundException);
174+
await expect(
175+
service.getUpdateFile({ filename: 'foo.txt' }),
176+
).rejects.toThrow(NotFoundException);
177+
});
178+
179+
it('throws NotFoundException when S3 returns NoSuchKey for a yml manifest', async () => {
180+
const error = new Error('Not found');
181+
error.name = 'NoSuchKey';
182+
mockSend.mockRejectedValue(error);
183+
184+
await expect(
185+
service.getUpdateFile({ filename: 'latest-mac.yml' }),
186+
).rejects.toThrow(NotFoundException);
187+
});
188+
});
189+
190+
describe('headUpdateFile', () => {
191+
it('returns metadata for .yml manifests', async () => {
192+
mockSend.mockResolvedValue({ ContentLength: 859 });
193+
194+
const result = await service.headUpdateFile({
195+
filename: 'latest-mac.yml',
196+
});
197+
198+
expect(result).toEqual({
199+
kind: 'stream',
200+
contentType: 'text/yaml',
201+
contentLength: 859,
202+
});
203+
expect(mockGetSignedUrl).not.toHaveBeenCalled();
204+
});
205+
206+
it('redirects binary HEAD requests to a URL signed with HeadObjectCommand', async () => {
207+
mockGetSignedUrl.mockResolvedValue('https://s3.example.com/signed-head');
208+
209+
const result = await service.headUpdateFile({
210+
filename: 'CompAI-Device-Agent-1.0.5-arm64.zip',
211+
});
212+
213+
expect(result).toEqual({
214+
kind: 'redirect',
215+
url: 'https://s3.example.com/signed-head',
216+
});
217+
expect(mockSend).not.toHaveBeenCalled();
218+
expect(mockGetSignedUrl).toHaveBeenCalledTimes(1);
219+
const [, command] = mockGetSignedUrl.mock.calls[0];
220+
// S3 signs each HTTP method separately; a GET-signed URL would be
221+
// rejected when used with a HEAD request.
222+
expect(command).toBeInstanceOf(MockHeadObjectCommand);
223+
expect(command).toMatchObject({
224+
Bucket: 'test-bucket',
225+
Key: 'device-agent/production/updates/CompAI-Device-Agent-1.0.5-arm64.zip',
226+
});
227+
});
228+
});
229+
95230
describe('downloadWindowsAgent', () => {
96231
it('should return stream, filename, and contentType on success', async () => {
97232
const mockStream = new Readable({ read() {} });

0 commit comments

Comments
 (0)