Skip to content

Commit 85a9ee7

Browse files
committed
fix: prefer .js over .ts when loading plugins for npm compatibility
- Add resolvePluginSource() helper (prefers .ts in dev, .js under node_modules) - Fix schema extraction to prefer .ts source for Rust parser - Fix fast-path plugin enable to call loadSource() before enabling - Root cause: schema extraction failed on compiled .js, config keys unrecognised, allowedDomains never set Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
1 parent 1e7cd06 commit 85a9ee7

File tree

4 files changed

+127
-17
lines changed

4 files changed

+127
-17
lines changed

src/agent/index.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import {
4646
contentHash,
4747
loadOperatorConfig,
4848
exceedsRiskThreshold,
49+
resolvePluginSource,
4950
} from "../plugin-system/manager.js";
5051
import { deepAudit, formatAuditResult } from "../plugin-system/auditor.js";
5152
import { extractSuggestedCommands } from "./command-suggestions.js";
@@ -701,11 +702,12 @@ if (discoveredCount > 0) {
701702
async function syncPluginsToSandbox(): Promise<void> {
702703
const enabled = pluginManager.getEnabledPlugins();
703704

704-
// Dynamic-import each enabled plugin's index.ts to get the register fn
705+
// Dynamic-import each enabled plugin to get the register fn
705706
const registrations = [];
706707
const loadErrors: string[] = [];
707708
for (const plugin of enabled) {
708-
const indexPath = join(plugin.dir, "index.ts");
709+
// Resolve .ts (dev) or .js (npm/dist) — centralised in plugin-system
710+
const indexPath = resolvePluginSource(plugin.dir);
709711

710712
// SECURITY CHECK 1: Verify source hasn't changed since audit/approval
711713
if (!pluginManager.verifySourceHash(plugin.manifest.name)) {

src/agent/slash-commands.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,6 +1207,11 @@ export async function handleSlashCommand(
12071207
break;
12081208
}
12091209

1210+
// Load source before enabling — syncPluginsToSandbox needs
1211+
// plugin.source for verifySourceHash(). Without this, the
1212+
// hash check fails and the plugin is silently disabled.
1213+
pluginManager.loadSource(pluginName);
1214+
12101215
pluginManager.enable(pluginName);
12111216
console.log(
12121217
` ✅ Plugin "${pluginName}" enabled (approved fast-path).`,

src/plugin-system/manager.ts

Lines changed: 62 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -308,22 +308,51 @@ export function contentHash(source: string): string {
308308
return createHash("sha256").update(source, "utf8").digest("hex");
309309
}
310310

311+
/**
312+
* Resolve the source file for a plugin directory.
313+
*
314+
* In dev mode (source repo), prefers .ts so edits are reflected immediately
315+
* without a rebuild. Under node_modules (npm install / bundled binary),
316+
* Node.js refuses to strip types from .ts files, so we always use .js.
317+
*/
318+
export function resolvePluginSource(pluginDir: string): string {
319+
const tsPath = join(pluginDir, "index.ts");
320+
const jsPath = join(pluginDir, "index.js");
321+
322+
// Under node_modules, Node.js can't type-strip .ts — always use .js.
323+
// Use path-segment check to avoid false positives on dirs named "node_modules_foo".
324+
const underNodeModules =
325+
/[\\/]node_modules[\\/]/.test(pluginDir) ||
326+
pluginDir.startsWith("node_modules/");
327+
if (underNodeModules) {
328+
return jsPath;
329+
}
330+
331+
// Dev mode: prefer .ts for live editing, fall back to .js
332+
return existsSync(tsPath) ? tsPath : jsPath;
333+
}
334+
311335
/**
312336
* Compute combined hash of plugin source and manifest.
313337
* Used for approval fingerprint and tamper detection.
314338
* Any change to either file invalidates the approval.
339+
*
340+
* Note: approvals are scoped to the install context. A plugin approved
341+
* in dev (from .ts) must be re-approved when loaded from node_modules
342+
* (.js), since the source content differs. This is intentional —
343+
* the compiled output should be verified independently.
315344
*/
316345
export function computePluginHash(pluginDir: string): string | null {
317-
const tsPath = join(pluginDir, "index.ts");
346+
const sourcePath = resolvePluginSource(pluginDir);
318347
const jsonPath = join(pluginDir, "plugin.json");
319348

320349
try {
321-
const tsContent = readFileSync(tsPath, "utf8");
350+
const sourceContent = readFileSync(sourcePath, "utf8");
322351
const jsonContent = readFileSync(jsonPath, "utf8");
323352

324353
// Hash both files together — any change invalidates
325354
return createHash("sha256")
326-
.update(tsContent, "utf8")
355+
.update(sourceContent, "utf8")
327356
.update(jsonContent, "utf8")
328357
.digest("hex");
329358
} catch {
@@ -648,16 +677,17 @@ export function createPluginManager(pluginsDir: string) {
648677
// ── Source Loading ────────────────────────────────────────────
649678

650679
/**
651-
* Load the source code of a plugin's index.js for auditing.
680+
* Load the source code of a plugin for auditing.
681+
* Resolves .ts (dev) or .js (npm/dist) via resolvePluginSource().
652682
* Returns the source string, or null if the file doesn't exist.
653683
*/
654684
function loadSource(name: string): string | null {
655685
const plugin = plugins.get(name);
656686
if (!plugin) return null;
657687

658-
const indexPath = join(plugin.dir, "index.ts");
688+
const indexPath = resolvePluginSource(plugin.dir);
659689
if (!existsSync(indexPath)) {
660-
console.error(`[plugins] Warning: ${name}/index.ts not found`);
690+
console.error(`[plugins] Warning: ${indexPath} not found`);
661691
return null;
662692
}
663693

@@ -667,7 +697,7 @@ export function createPluginManager(pluginsDir: string) {
667697
return source;
668698
} catch (err) {
669699
console.error(
670-
`[plugins] Warning: failed to read ${name}/index.ts: ${(err as Error).message}`,
700+
`[plugins] Warning: failed to read ${name} source: ${(err as Error).message}`,
671701
);
672702
return null;
673703
}
@@ -688,10 +718,27 @@ export function createPluginManager(pluginsDir: string) {
688718
const plugin = plugins.get(name);
689719
if (!plugin) return false;
690720

691-
// Load source if not already loaded
721+
// For schema extraction, prefer .ts source — it's the canonical
722+
// schema definition and the Rust parser handles it best. Under
723+
// node_modules, only .js exists so we fall back gracefully.
724+
// try/catch guards against TOCTOU (file deleted between exists check and read).
725+
const tsPath = join(plugin.dir, "index.ts");
726+
let extractionSource: string | null = null;
727+
if (existsSync(tsPath)) {
728+
try {
729+
extractionSource = readFileSync(tsPath, "utf8");
730+
} catch {
731+
// Fall through to plugin.source / loadSource
732+
}
733+
}
734+
if (!extractionSource) {
735+
extractionSource = plugin.source ?? loadSource(name);
736+
}
737+
if (!extractionSource) return false;
738+
739+
// Also ensure plugin.source is loaded for hash verification
692740
if (!plugin.source) {
693-
const source = loadSource(name);
694-
if (!source) return false;
741+
loadSource(name);
695742
}
696743

697744
// If analysis guest is not enabled, fall back to manifest.
@@ -705,7 +752,7 @@ export function createPluginManager(pluginsDir: string) {
705752
}
706753

707754
try {
708-
const metadata = await extractPluginMetadata(plugin.source!);
755+
const metadata = await extractPluginMetadata(extractionSource);
709756

710757
// Use extracted schema or fall back to manifest
711758
if (metadata.schema) {
@@ -804,7 +851,7 @@ export function createPluginManager(pluginsDir: string) {
804851
* Approve a plugin. Requires an existing audit result.
805852
* Persists the approval to disk immediately.
806853
*
807-
* Uses combined hash (index.ts + plugin.json) for tamper detection.
854+
* Uses combined hash (plugin source + plugin.json) for tamper detection.
808855
*
809856
* @returns true if approved, false if plugin not found or not audited
810857
*/
@@ -851,7 +898,7 @@ export function createPluginManager(pluginsDir: string) {
851898

852899
/**
853900
* Check if a plugin has a valid, current approval.
854-
* Compares the stored content hash against combined hash (index.ts + plugin.json).
901+
* Compares the stored content hash against combined hash (plugin source + plugin.json).
855902
*/
856903
function isApproved(name: string): boolean {
857904
const plugin = plugins.get(name);
@@ -869,7 +916,7 @@ export function createPluginManager(pluginsDir: string) {
869916

870917
/**
871918
* Refresh the `approved` flag on all plugins based on the
872-
* persisted approval store and current combined hash (index.ts + plugin.json).
919+
* persisted approval store and current combined hash (plugin source + plugin.json).
873920
* Called after discover() to sync runtime flags with disk state.
874921
*/
875922
function refreshAllApprovals(): void {
@@ -1167,7 +1214,7 @@ export function createPluginManager(pluginsDir: string) {
11671214
const plugin = plugins.get(name);
11681215
if (!plugin || !plugin.source) return false;
11691216

1170-
const indexPath = join(plugin.dir, "index.ts");
1217+
const indexPath = resolvePluginSource(plugin.dir);
11711218
try {
11721219
const currentSource = readFileSync(indexPath, "utf8");
11731220
return currentSource === plugin.source;

tests/plugin-manager.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
coerceConfigValue,
2525
loadOperatorConfig,
2626
exceedsRiskThreshold,
27+
resolvePluginSource,
2728
} from "../src/plugin-system/manager.js";
2829

2930
// ── Fixtures path ────────────────────────────────────────────────────
@@ -446,6 +447,61 @@ describe("contentHash", () => {
446447
});
447448
});
448449

450+
// ── resolvePluginSource ──────────────────────────────────────────────
451+
452+
describe("resolvePluginSource", () => {
453+
// Create temp fixtures with controlled file combinations
454+
const tmpBase = join(
455+
dirname(fileURLToPath(import.meta.url)),
456+
"..",
457+
"tmp-resolve-test-" + process.pid,
458+
);
459+
const devDir = join(tmpBase, "dev-plugin");
460+
const devJsOnly = join(tmpBase, "dev-js-only");
461+
const nmDir = join(tmpBase, "node_modules", "test-plugin");
462+
463+
beforeEach(async () => {
464+
const { mkdirSync, writeFileSync: fsWrite } = await import("node:fs");
465+
mkdirSync(devDir, { recursive: true });
466+
fsWrite(join(devDir, "index.ts"), "export const x = 1;");
467+
fsWrite(join(devDir, "index.js"), "exports.x = 1;");
468+
469+
mkdirSync(devJsOnly, { recursive: true });
470+
fsWrite(join(devJsOnly, "index.js"), "exports.x = 1;");
471+
472+
mkdirSync(nmDir, { recursive: true });
473+
fsWrite(join(nmDir, "index.ts"), "export const x = 1;");
474+
fsWrite(join(nmDir, "index.js"), "exports.x = 1;");
475+
});
476+
477+
afterEach(async () => {
478+
const { rmSync } = await import("node:fs");
479+
rmSync(tmpBase, { recursive: true, force: true });
480+
});
481+
482+
it("should prefer .ts over .js in dev (non-node_modules) dirs", () => {
483+
const result = resolvePluginSource(devDir);
484+
expect(result).toMatch(/index\.ts$/);
485+
});
486+
487+
it("should fall back to .js when .ts does not exist in dev", () => {
488+
const result = resolvePluginSource(devJsOnly);
489+
expect(result).toMatch(/index\.js$/);
490+
});
491+
492+
it("should always return .js under node_modules paths", () => {
493+
const result = resolvePluginSource(nmDir);
494+
expect(result).toMatch(/index\.js$/);
495+
});
496+
497+
it("should return .js path even if .js missing under node_modules", () => {
498+
unlinkSync(join(nmDir, "index.js"));
499+
const result = resolvePluginSource(nmDir);
500+
// Must return .js (not .ts) — Node can't strip types under node_modules
501+
expect(result).toMatch(/index\.js$/);
502+
});
503+
});
504+
449505
// ── Plugin Manager ───────────────────────────────────────────────────
450506

451507
describe("createPluginManager", () => {

0 commit comments

Comments
 (0)