Skip to content

Commit 05b49de

Browse files
committed
fix: address plugin marketplace review feedback and split the mail plugin out of tree
1 parent 6373d0c commit 05b49de

54 files changed

Lines changed: 875 additions & 2377 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ const marketplaceScript = join(repoRoot, 'scripts', 'dev-marketplace.mts');
1717
const fixtureIndex = join(repoRoot, 'node_modules', '.cache', 'dev-marketplace', 'index.json');
1818

1919
const MARKETPLACE_ID = 'linkcode-official';
20-
const PLUGIN_ID = 'linkcode/mail';
20+
const PLUGIN_ID = 'linkcode/echo';
2121
const PLUGIN_VERSION = '0.1.0';
22-
const AUTHCODE = 'e2e-secret-authcode';
22+
const SECRET_TOKEN = 'e2e-secret-token';
2323

2424
async function freePort(): Promise<number> {
2525
const server = createServer();
@@ -118,7 +118,7 @@ async function main(): Promise<void> {
118118
(entry) =>
119119
entry.pluginId === PLUGIN_ID && entry.release.manifest.version === PLUGIN_VERSION,
120120
),
121-
'catalog does not list linkcode/mail',
121+
'catalog does not list linkcode/echo',
122122
);
123123
const second = await client.refreshPluginMarketplace(MARKETPLACE_ID);
124124
assert.equal(second.notModified, true, 'second refresh did not hit the ETag cache');
@@ -137,45 +137,45 @@ async function main(): Promise<void> {
137137
version: PLUGIN_VERSION,
138138
});
139139
assert.equal(installed.pluginId, PLUGIN_ID);
140-
const packageDir = join(home, '.linkcode', 'plugins', 'linkcode', 'mail', PLUGIN_VERSION);
140+
const packageDir = join(home, '.linkcode', 'plugins', 'linkcode', 'echo', PLUGIN_VERSION);
141141
assert(existsSync(join(packageDir, 'manifest.json')), 'installed manifest.json missing');
142142
assert(existsSync(join(packageDir, 'dist', 'index.js')), 'installed dist/index.js missing');
143143

144144
// 4. Settings: masked read shows the schema, set splits secret vs non-secret.
145145
const before = await client.listLinkCodePluginConfigs();
146146
const view = before.find((entry) => entry.id === PLUGIN_ID);
147147
assert(view, 'installed plugin missing from plugin-config.list');
148-
assert(view.settings.authcode?.secret, 'authcode must be a secret field');
149-
assert.equal(view.values.authcode, undefined, 'secret value leaked in masked read');
148+
assert(view.settings.token?.secret, 'token must be a secret field');
149+
assert.equal(view.values.token, undefined, 'secret value leaked in masked read');
150150

151151
await client.setLinkCodePluginConfig({
152152
pluginId: PLUGIN_ID,
153-
set: { account: 'user@163.com', authcode: AUTHCODE, preset: 'qq' },
153+
set: { greeting: '你好', token: SECRET_TOKEN, mode: 'shout' },
154154
});
155155
const configFile = JSON.parse(readFileSync(join(home, '.linkcode', 'config.json'), 'utf8')) as {
156156
pluginConfigs?: Record<string, Record<string, unknown>>;
157157
};
158-
assert.equal(configFile.pluginConfigs?.[PLUGIN_ID]?.account, 'user@163.com');
159-
assert.equal(configFile.pluginConfigs?.[PLUGIN_ID]?.preset, 'qq');
160-
assert(!('authcode' in (configFile.pluginConfigs?.[PLUGIN_ID] ?? {})), 'secret in config.json');
158+
assert.equal(configFile.pluginConfigs?.[PLUGIN_ID]?.greeting, '你好');
159+
assert.equal(configFile.pluginConfigs?.[PLUGIN_ID]?.mode, 'shout');
160+
assert(!('token' in (configFile.pluginConfigs?.[PLUGIN_ID] ?? {})), 'secret in config.json');
161161
const secretsFile = JSON.parse(
162162
readFileSync(join(home, '.linkcode', 'secrets.json'), 'utf8'),
163163
) as {
164164
protection: 'os-keyring' | 'plaintext';
165165
};
166166
// A fake HOME has no login keychain, so the vault degrades to plaintext on disk (with a boot
167-
// warning). Either way the authcode belongs in secrets.json — just never in config.json.
167+
// warning). Either way the token belongs in secrets.json — just never in config.json.
168168
const secretsRaw = readFileSync(join(home, '.linkcode', 'secrets.json'), 'utf8');
169169
if (secretsFile.protection === 'os-keyring') {
170-
assert(!secretsRaw.includes(AUTHCODE), 'authcode stored in plaintext under os-keyring');
170+
assert(!secretsRaw.includes(SECRET_TOKEN), 'token stored in plaintext under os-keyring');
171171
} else {
172-
assert(secretsRaw.includes(AUTHCODE), 'authcode missing from the vault');
172+
assert(secretsRaw.includes(SECRET_TOKEN), 'token missing from the vault');
173173
}
174174

175175
const after = await client.listLinkCodePluginConfigs();
176176
const afterView = after.find((entry) => entry.id === PLUGIN_ID);
177-
assert.equal(afterView?.values.account, 'user@163.com');
178-
assert.equal(afterView?.values.authcode, undefined, 'secret value leaked after set');
177+
assert.equal(afterView?.values.greeting, '你好');
178+
assert.equal(afterView?.values.token, undefined, 'secret value leaked after set');
179179

180180
// 5. Uninstall removes the package and prunes its config.
181181
const removed = await client.uninstallLinkCodePlugin(PLUGIN_ID);

‎apps/daemon/src/__tests__/fixtures/in-memory-vault.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,13 @@ export function createInMemoryVault(protection: SecretProtection = 'os-keyring')
2020
return {
2121
protection,
2222
get: (key) => refs.get(prefix + key) ?? null,
23+
keys: () => {
24+
const keys: string[] = [];
25+
for (const ref of refs.keys()) {
26+
if (ref.startsWith(prefix)) keys.push(ref.slice(prefix.length));
27+
}
28+
return keys;
29+
},
2330
set(key, secret) {
2431
refs.set(prefix + key, secret);
2532
},

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,12 +156,18 @@ describe('DaemonLinkCodeMarketplaceService.refresh', () => {
156156
});
157157

158158
it('does not refresh or resolve releases from a disabled marketplace', async () => {
159+
// Populate the cache through an enabled config first, so the resolveRelease assertion below
160+
// actually exercises the disabled gate instead of short-circuiting on an empty cache.
161+
const fetchIndex = vi.fn(() => Promise.resolve(fakeResponse(200, JSON.stringify(INDEX))));
162+
await new DaemonLinkCodeMarketplaceService(MARKETPLACES, fetchIndex).refresh(
163+
'linkcode-official',
164+
);
165+
159166
const disabled: LinkCodeMarketplaceConfigList = [{ ...MARKETPLACES[0], enabled: false }];
160-
const fetchIndex = vi.fn();
161167
const service = new DaemonLinkCodeMarketplaceService(disabled, fetchIndex);
162168

163169
await expect(service.refresh('linkcode-official')).rejects.toThrow('Marketplace is disabled');
164-
expect(fetchIndex).not.toHaveBeenCalled();
170+
expect(fetchIndex).toHaveBeenCalledTimes(1);
165171
expect(
166172
service.resolveRelease({
167173
marketplaceId: 'linkcode-official',

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

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type {
66
LinkCodePluginManifest,
77
LinkCodePluginRelease,
88
} from '@linkcode/schema';
9+
import { wait } from 'foxts/wait';
910
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
1011
import { makePluginTmpDir, pluginPackageDir, pluginRegistryPath } from '../plugin-store/paths';
1112
import { DaemonLinkCodePluginStore } from '../plugin-store/store';
@@ -128,6 +129,85 @@ describe('DaemonLinkCodePluginStore', () => {
128129
]);
129130
});
130131

132+
it('serializes concurrent installs of the same plugin so neither deletes the other’s package', async () => {
133+
const events: string[] = [];
134+
mocks.tarExtract.mockImplementation(async ({ cwd }: { cwd: string }) => {
135+
events.push('extract:start');
136+
await wait(10);
137+
writeFileSync(join(cwd, 'manifest.json'), JSON.stringify(manifest('0.2.0')));
138+
events.push('extract:end');
139+
});
140+
const release = {
141+
manifest: manifest('0.2.0'),
142+
artifact: {
143+
urls: ['https://plugins.example/arcbox-latex-0.2.0.tgz'],
144+
integrity: 'sha256-7bZ8YaunaCifbaRByeb1I8+v9PiypXCFI+8pxUP46I4=',
145+
format: 'tgz',
146+
},
147+
} satisfies LinkCodePluginRelease;
148+
const store = new DaemonLinkCodePluginStore(createInMemoryVault());
149+
150+
await Promise.all([
151+
store.install(release, 'linkcode-official'),
152+
store.install(release, 'linkcode-official'),
153+
]);
154+
155+
expect(events).toEqual(['extract:start', 'extract:end', 'extract:start', 'extract:end']);
156+
expect(existsSync(pluginPackageDir('arcbox/latex', '0.2.0'))).toBe(true);
157+
expect(JSON.parse(readFileSync(pluginRegistryPath(), 'utf8'))).toMatchObject([
158+
{ id: 'arcbox/latex', version: '0.2.0' },
159+
]);
160+
});
161+
162+
it('uninstall prunes only its own secrets, even beside a dotted sibling id', async () => {
163+
const installed = record('0.1.0');
164+
writePackage(installed, settingsManifest('0.1.0'));
165+
const neighbour: InstalledLinkCodePlugin = {
166+
...record('0.3.0'),
167+
// Dots are legal inside id segments, so `arcbox/latex.pro` is a real neighbour whose keys
168+
// would match a naive `arcbox/latex.` prefix; a corrupt manifest must not turn them into
169+
// prunable orphans either.
170+
id: 'arcbox/latex.pro',
171+
path: pluginPackageDir('arcbox/latex.pro', '0.3.0'),
172+
};
173+
writePackage(neighbour, '{broken');
174+
writeRegistry([installed, neighbour]);
175+
const vault = createInMemoryVault();
176+
const store = new DaemonLinkCodePluginStore(vault);
177+
await store.setSettings('arcbox/latex', {
178+
set: { account: 'a@example.com', authcode: 'secret-a' },
179+
});
180+
vault.namespace('plugin').set('arcbox/latex.pro/authcode', 'secret-b');
181+
182+
await store.uninstall('arcbox/latex');
183+
184+
const secrets = vault.namespace('plugin');
185+
expect(secrets.get('arcbox/latex/authcode')).toBeNull();
186+
expect(secrets.get('arcbox/latex.pro/authcode')).toBe('secret-b');
187+
});
188+
189+
it('folds manifest defaults into settings that have no stored value', async () => {
190+
const installed = record('0.1.0');
191+
const withDefaults = settingsManifest('0.1.0');
192+
withDefaults.settings = {
193+
...withDefaults.settings,
194+
preset: { type: 'enum', enum: ['163', 'qq'], default: '163' },
195+
limit: { type: 'number', default: 8000 },
196+
fallbacktoken: { type: 'password', secret: true, default: 'manifest-leak' },
197+
};
198+
writePackage(installed, withDefaults);
199+
writeRegistry([installed]);
200+
const store = new DaemonLinkCodePluginStore(createInMemoryVault());
201+
await store.setSettings('arcbox/latex', { set: { account: 'a@example.com' } });
202+
203+
// A secret field's default is a plaintext credential in the manifest — never folded in.
204+
expect(store.getSettings('arcbox/latex')).toEqual({
205+
account: 'a@example.com',
206+
preset: '163',
207+
limit: 8000,
208+
});
209+
});
210+
131211
it('rolls back config and secret changes when the vault rejects a settings update', () => {
132212
const installed = record('0.1.0');
133213
writePackage(installed, settingsManifest('0.1.0'));
@@ -146,7 +226,7 @@ describe('DaemonLinkCodePluginStore', () => {
146226
return {
147227
...secrets,
148228
set(key: string, value: string) {
149-
if (key === 'arcbox/latex.authcode' && value === 'new-secret') throw vaultFailure;
229+
if (key === 'arcbox/latex/authcode' && value === 'new-secret') throw vaultFailure;
150230
secrets.set(key, value);
151231
},
152232
};

‎apps/daemon/src/config.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ export function savePluginConfigValues(
324324
writeConfigFields(file, { pluginConfigs: configs });
325325
}
326326

327-
/** The daemon's `plugin` vault namespace, for secret setting values keyed `<pluginId>.<fieldId>`. */
327+
/** The daemon's `plugin` vault namespace, for secret setting values keyed `<pluginId>/<fieldId>`. */
328328
export function pluginSecretStore(vault: SecretVault): SecretStore {
329329
return pluginSecrets(vault);
330330
}

0 commit comments

Comments
 (0)