Skip to content

Commit 6373d0c

Browse files
adminliu-mainadminliu-main
committed
fix(daemon): harden plugin install staging, settings rollback, and marketplace refresh
1 parent 2509c6c commit 6373d0c

19 files changed

Lines changed: 658 additions & 45 deletions

File tree

‎apps/daemon/e2e/plugin-marketplace.e2e.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,13 @@ async function main(): Promise<void> {
122122
);
123123
const second = await client.refreshPluginMarketplace(MARKETPLACE_ID);
124124
assert.equal(second.notModified, true, 'second refresh did not hit the ETag cache');
125+
assert(
126+
second.releases.some(
127+
(entry) =>
128+
entry.pluginId === PLUGIN_ID && entry.release.manifest.version === PLUGIN_VERSION,
129+
),
130+
'304 refresh cleared the cached catalog',
131+
);
125132

126133
// 3. Install from the cached catalog; the package lands in the Store.
127134
const installed = await client.installLinkCodePlugin({

‎apps/daemon/src/__tests__/marketplace.test.ts‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import { mkdtempSync } from 'node:fs';
1+
import { mkdtempSync, writeFileSync } from 'node:fs';
22
import { tmpdir } from 'node:os';
33
import { join } from 'node:path';
44
import type { LinkCodeMarketplaceConfigList } from '@linkcode/schema';
55
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
6+
import { marketplaceIndexCachePath } from '../marketplace/paths';
67
import type { MarketplaceIndexResponse } from '../marketplace/service';
78
import { DaemonLinkCodeMarketplaceService } from '../marketplace/service';
89

@@ -128,6 +129,48 @@ describe('DaemonLinkCodeMarketplaceService.refresh', () => {
128129
).toBeDefined();
129130
});
130131

132+
it('drops stale validators and retries unconditionally when a 304 has no readable cache', async () => {
133+
let calls = 0;
134+
const fetchIndex = vi.fn(() => {
135+
calls += 1;
136+
return Promise.resolve(
137+
calls === 1
138+
? fakeResponse(200, JSON.stringify(INDEX), { etag: '"index-v1"' })
139+
: calls === 2
140+
? fakeResponse(304)
141+
: fakeResponse(200, JSON.stringify(INDEX), { etag: '"index-v2"' }),
142+
);
143+
});
144+
const service = new DaemonLinkCodeMarketplaceService(MARKETPLACES, fetchIndex);
145+
146+
await service.refresh('linkcode-official');
147+
writeFileSync(marketplaceIndexCachePath('linkcode-official'), '{broken', 'utf8');
148+
const result = await service.refresh('linkcode-official');
149+
150+
expect(result.releases).toHaveLength(1);
151+
expect(fetchIndex).toHaveBeenNthCalledWith(
152+
3,
153+
'https://plugins.example/index.json',
154+
expect.objectContaining({ headers: {} }),
155+
);
156+
});
157+
158+
it('does not refresh or resolve releases from a disabled marketplace', async () => {
159+
const disabled: LinkCodeMarketplaceConfigList = [{ ...MARKETPLACES[0], enabled: false }];
160+
const fetchIndex = vi.fn();
161+
const service = new DaemonLinkCodeMarketplaceService(disabled, fetchIndex);
162+
163+
await expect(service.refresh('linkcode-official')).rejects.toThrow('Marketplace is disabled');
164+
expect(fetchIndex).not.toHaveBeenCalled();
165+
expect(
166+
service.resolveRelease({
167+
marketplaceId: 'linkcode-official',
168+
pluginId: 'arcbox/latex',
169+
version: '1.2.0',
170+
}),
171+
).toBeUndefined();
172+
});
173+
131174
it('discards cached validators when the configured source URL changed', async () => {
132175
const fetchIndex = vi.fn(() =>
133176
Promise.resolve(fakeResponse(200, JSON.stringify(INDEX), { etag: '"index-v1"' })),

‎apps/daemon/src/__tests__/plugin-store.test.ts‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
import { existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs';
2+
import { tmpdir } from 'node:os';
3+
import { join } from 'node:path';
4+
import type {
5+
InstalledLinkCodePlugin,
6+
LinkCodePluginManifest,
7+
LinkCodePluginRelease,
8+
} from '@linkcode/schema';
9+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
10+
import { makePluginTmpDir, pluginPackageDir, pluginRegistryPath } from '../plugin-store/paths';
11+
import { DaemonLinkCodePluginStore } from '../plugin-store/store';
12+
import { createInMemoryVault } from './fixtures/in-memory-vault';
13+
14+
const mocks = vi.hoisted(() => ({
15+
downloadVerified: vi.fn(),
16+
tarExtract: vi.fn(),
17+
}));
18+
19+
vi.mock('@linkcode/assets', () => ({ downloadVerified: mocks.downloadVerified }));
20+
vi.mock('tar', () => ({ extract: mocks.tarExtract }));
21+
22+
let savedHome: string | undefined;
23+
24+
beforeEach(() => {
25+
savedHome = process.env.HOME;
26+
process.env.HOME = mkdtempSync(join(tmpdir(), 'linkcode-plugin-store-'));
27+
process.env.LINKCODE_CHANNEL = 'release';
28+
mocks.downloadVerified.mockReset().mockResolvedValue(undefined);
29+
mocks.tarExtract.mockReset();
30+
});
31+
32+
afterEach(() => {
33+
process.env.HOME = savedHome;
34+
delete process.env.LINKCODE_CHANNEL;
35+
vi.restoreAllMocks();
36+
});
37+
38+
function manifest(version: string, componentName = 'latex'): LinkCodePluginManifest {
39+
return {
40+
manifestVersion: 1,
41+
id: 'arcbox/latex',
42+
version,
43+
keywords: [],
44+
components: [{ kind: 'skill', name: componentName, entry: 'skills/latex/SKILL.md' }],
45+
assets: [],
46+
};
47+
}
48+
49+
function record(version: string): InstalledLinkCodePlugin {
50+
return {
51+
id: 'arcbox/latex',
52+
version,
53+
marketplaceId: 'linkcode-official',
54+
integrity: 'sha256-7bZ8YaunaCifbaRByeb1I8+v9PiypXCFI+8pxUP46I4=',
55+
enabled: true,
56+
path: pluginPackageDir('arcbox/latex', version),
57+
};
58+
}
59+
60+
function writePackage(installed: InstalledLinkCodePlugin, packageManifest: unknown): void {
61+
mkdirSync(installed.path, { recursive: true });
62+
writeFileSync(join(installed.path, 'manifest.json'), JSON.stringify(packageManifest));
63+
}
64+
65+
function writeRegistry(records: InstalledLinkCodePlugin[]): void {
66+
const path = pluginRegistryPath();
67+
mkdirSync(join(path, '..'), { recursive: true });
68+
writeFileSync(path, JSON.stringify(records));
69+
}
70+
71+
function settingsManifest(version: string): LinkCodePluginManifest {
72+
return {
73+
...manifest(version),
74+
settings: {
75+
account: { type: 'string', label: 'Account' },
76+
authcode: { type: 'password', label: 'Authorization code', secret: true },
77+
},
78+
};
79+
}
80+
81+
describe('DaemonLinkCodePluginStore', () => {
82+
it('allocates a unique staging directory for concurrent installs of the same release', () => {
83+
expect(makePluginTmpDir('arcbox/latex', '0.2.0')).not.toBe(
84+
makePluginTmpDir('arcbox/latex', '0.2.0'),
85+
);
86+
});
87+
88+
it('uses the most recently installed record for legacy duplicate plugin ids', () => {
89+
const v1 = record('0.1.0');
90+
const v2 = record('0.2.0');
91+
writePackage(v1, { ...manifest('0.1.0'), futureManifestField: 'ignored' });
92+
writePackage(v2, manifest('0.2.0'));
93+
writeRegistry([v1, v2]);
94+
95+
const store = new DaemonLinkCodePluginStore(createInMemoryVault());
96+
97+
expect(store.list()).toMatchObject([{ installed: { version: '0.2.0' } }]);
98+
expect(store.get('arcbox/latex')?.installed.version).toBe('0.2.0');
99+
});
100+
101+
it('replaces an older package and returns the verified on-disk manifest', async () => {
102+
const v0 = record('0.0.1');
103+
const v1 = record('0.1.0');
104+
writePackage(v0, manifest('0.0.1'));
105+
writePackage(v1, manifest('0.1.0'));
106+
writeRegistry([v0, v1]);
107+
mocks.tarExtract.mockImplementation(({ cwd }: { cwd: string }) => {
108+
writeFileSync(join(cwd, 'manifest.json'), JSON.stringify(manifest('0.2.0', 'package-skill')));
109+
});
110+
const release = {
111+
manifest: manifest('0.2.0', 'index-skill'),
112+
artifact: {
113+
urls: ['https://plugins.example/arcbox-latex-0.2.0.tgz'],
114+
integrity: 'sha256-7bZ8YaunaCifbaRByeb1I8+v9PiypXCFI+8pxUP46I4=',
115+
format: 'tgz',
116+
},
117+
} satisfies LinkCodePluginRelease;
118+
const store = new DaemonLinkCodePluginStore(createInMemoryVault());
119+
120+
const installed = await store.install(release, 'linkcode-official');
121+
122+
expect(installed.manifest.components[0]?.name).toBe('package-skill');
123+
expect(store.get('arcbox/latex')?.manifest.components[0]?.name).toBe('package-skill');
124+
expect(existsSync(v0.path)).toBe(false);
125+
expect(existsSync(v1.path)).toBe(false);
126+
expect(JSON.parse(readFileSync(pluginRegistryPath(), 'utf8'))).toMatchObject([
127+
{ id: 'arcbox/latex', version: '0.2.0' },
128+
]);
129+
});
130+
131+
it('rolls back config and secret changes when the vault rejects a settings update', () => {
132+
const installed = record('0.1.0');
133+
writePackage(installed, settingsManifest('0.1.0'));
134+
writeRegistry([installed]);
135+
const baseVault = createInMemoryVault();
136+
const store = new DaemonLinkCodePluginStore(baseVault);
137+
store.setSettings('arcbox/latex', {
138+
set: { account: 'old@example.com', authcode: 'old-secret' },
139+
});
140+
const vaultFailure = new Error('vault unavailable');
141+
const flakyVault = {
142+
...baseVault,
143+
namespace(name: Parameters<typeof baseVault.namespace>[0]) {
144+
const secrets = baseVault.namespace(name);
145+
if (name !== 'plugin') return secrets;
146+
return {
147+
...secrets,
148+
set(key: string, value: string) {
149+
if (key === 'arcbox/latex.authcode' && value === 'new-secret') throw vaultFailure;
150+
secrets.set(key, value);
151+
},
152+
};
153+
},
154+
};
155+
156+
expect(() =>
157+
new DaemonLinkCodePluginStore(flakyVault).setSettings('arcbox/latex', {
158+
set: { account: 'new@example.com', authcode: 'new-secret' },
159+
}),
160+
).toThrow(vaultFailure);
161+
162+
expect(store.getSettings('arcbox/latex')).toEqual({
163+
account: 'old@example.com',
164+
authcode: 'old-secret',
165+
});
166+
});
167+
});

‎apps/daemon/src/marketplace/service.ts‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,18 @@ export class DaemonLinkCodeMarketplaceService implements LinkCodeMarketplaceServ
6363
}
6464

6565
async refresh(marketplaceId: string): Promise<MarketplaceRefreshResult> {
66+
return this.refreshIndex(marketplaceId, false);
67+
}
68+
69+
private async refreshIndex(
70+
marketplaceId: string,
71+
retriedWithoutValidators: boolean,
72+
): Promise<MarketplaceRefreshResult> {
6673
const config = nullthrow(
6774
this.marketplaces.find((entry) => entry.id === marketplaceId),
6875
`Unknown marketplace: ${marketplaceId}`,
6976
);
77+
if (!config.enabled) throw new Error(`Marketplace is disabled: ${marketplaceId}`);
7078
const url = config.source.url;
7179
// Validators are only replayed against the exact URL that produced them.
7280
const state = readRefreshState(marketplaceId);
@@ -81,15 +89,28 @@ export class DaemonLinkCodeMarketplaceService implements LinkCodeMarketplaceServ
8189
signal: AbortSignal.timeout(REFRESH_TIMEOUT_MS),
8290
});
8391
if (response.status === 304) {
92+
const cachedIndex = readIndexCache(marketplaceId);
93+
if (cachedIndex === undefined) {
94+
if (retriedWithoutValidators) {
95+
throw new Error('Marketplace returned HTTP 304 without a usable cached index');
96+
}
97+
// A validator is only meaningful alongside the index it validates. If local state was
98+
// deleted or corrupted, remove it and retry once without conditional request headers.
99+
dropCachedIndexAndValidators(marketplaceId);
100+
logger.warn(
101+
{ marketplaceId, operation: 'marketplace.refresh' },
102+
'Received HTTP 304 without a usable cached index; retrying unconditionally',
103+
);
104+
return this.refreshIndex(marketplaceId, true);
105+
}
84106
if (validators !== undefined) {
85107
writeRefreshState({ ...validators, checkedAt: Date.now() });
86108
}
87109
// A 304 means the remote index is unchanged, not that the catalog is empty. Reuse the
88110
// daemon's persisted index so clients can replace their snapshot safely even when they do not
89111
// retain the previous response in memory (for example after an uninstall or page remount).
90-
const cachedIndex = readIndexCache(marketplaceId);
91112
return {
92-
releases: cachedIndex === undefined ? [] : flattenReleases(cachedIndex),
113+
releases: flattenReleases(cachedIndex),
93114
notModified: true,
94115
};
95116
}
@@ -113,7 +134,7 @@ export class DaemonLinkCodeMarketplaceService implements LinkCodeMarketplaceServ
113134
resolveRelease(identity: LinkCodeMarketplaceReleaseIdentity): LinkCodePluginRelease | undefined {
114135
const config = this.marketplaces.find((entry) => entry.id === identity.marketplaceId);
115136
const index = readIndexCache(identity.marketplaceId);
116-
if (config === undefined || index === undefined) return undefined;
137+
if (index === undefined || !config?.enabled) return undefined;
117138
const plugin = index.plugins.find((entry) => entry.id === identity.pluginId);
118139
const release = plugin?.releases.find(
119140
(candidate) => candidate.manifest.version === identity.version,
@@ -129,6 +150,11 @@ export class DaemonLinkCodeMarketplaceService implements LinkCodeMarketplaceServ
129150
}
130151
}
131152

153+
function dropCachedIndexAndValidators(marketplaceId: string): void {
154+
rmSync(marketplaceIndexCachePath(marketplaceId), { force: true });
155+
rmSync(marketplaceRefreshStatePath(marketplaceId), { force: true });
156+
}
157+
132158
function resolveMirrorUrl(url: string, indexUrl: string): string {
133159
if (ABSOLUTE_HTTP_URL_RE.test(url)) return url;
134160
return new URL(url, indexUrl).href;

‎apps/daemon/src/plugin-store/paths.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { randomUUID } from 'node:crypto';
12
import { mkdirSync } from 'node:fs';
23
import { homedir } from 'node:os';
34
import { join } from 'node:path';
@@ -28,12 +29,13 @@ export function pluginPackageDir(pluginId: string, version: string): string {
2829
return join(pluginsRoot(), ...safe, version);
2930
}
3031

31-
/** Staging dir beside the package dir, so publish is one same-volume `rename`. */
32+
/** Unique staging dir beside the package dir, so concurrent installs publish through one same-volume
33+
* `rename` without sharing a partially extracted archive. */
3234
export function makePluginTmpDir(pluginId: string, version: string): string {
3335
const dir = pluginPackageDir(pluginId, version);
3436
const parent = join(dir, '..');
3537
mkdirSync(dir, { recursive: true });
36-
return join(parent, `.tmp-${process.pid}-${version}`);
38+
return join(parent, `.tmp-${process.pid}-${version}-${randomUUID()}`);
3739
}
3840

3941
/** Resolve product channel for callers that must not reach into the paths module's side effects. */

0 commit comments

Comments
 (0)