Skip to content

Commit 5922b1f

Browse files
committed
fix(runtime): keep caller security precheck authoritative
1 parent bbf04c6 commit 5922b1f

2 files changed

Lines changed: 119 additions & 162 deletions

File tree

packages/runtime/src/embed.ts

Lines changed: 15 additions & 162 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
1-
import {
2-
collectRuntimeSourceImports,
3-
type RuntimeEvent,
4-
type RuntimePlan,
5-
type RuntimeStateSnapshot,
1+
import type {
2+
RuntimeEvent,
3+
RuntimePlan,
4+
RuntimeStateSnapshot,
65
} from "@renderify/ir";
76
import {
87
DefaultSecurityChecker,
98
type RuntimeSecurityProfile,
10-
type SecurityCheckResult,
119
type SecurityInitializationOptions,
1210
} from "@renderify/security";
1311
import { JspmModuleLoader } from "./jspm-module-loader";
@@ -352,21 +350,6 @@ async function precheckPlanBeforeAutoPin(
352350
}
353351

354352
const policy = security.getPolicy();
355-
if (callerProvidedSecurity) {
356-
const callerPrecheckResult = await filterAutoPinnablePrecheckIssues(
357-
await security.checkPlan(plan),
358-
plan,
359-
policy,
360-
);
361-
if (!callerPrecheckResult.safe) {
362-
return callerPrecheckResult;
363-
}
364-
365-
if (!policy.requireModuleManifestForBareSpecifiers) {
366-
return callerPrecheckResult;
367-
}
368-
}
369-
370353
if (!policy.requireModuleManifestForBareSpecifiers) {
371354
return security.checkPlan(plan);
372355
}
@@ -380,157 +363,27 @@ async function precheckPlanBeforeAutoPin(
380363
},
381364
});
382365

383-
return precheckSecurity.checkPlan(plan);
384-
}
385-
386-
async function filterAutoPinnablePrecheckIssues(
387-
result: SecurityCheckResult,
388-
plan: RuntimePlan,
389-
policy: ReturnType<
390-
NonNullable<RuntimeEmbedRenderOptions["security"]>["getPolicy"]
391-
>,
392-
): Promise<SecurityCheckResult> {
393-
if (result.safe || !policy.requireModuleManifestForBareSpecifiers) {
394-
return result;
395-
}
396-
397-
const autoPinnableSpecifiers = await collectAutoPinnableBareSpecifiers(plan);
398-
if (autoPinnableSpecifiers.size === 0) {
399-
return result;
400-
}
401-
402-
const removedIssues = new Set<string>();
403-
const remainingIssues = result.issues.filter((issue) => {
404-
const shouldRemove = isAutoPinnablePrecheckIssue(
405-
issue,
406-
autoPinnableSpecifiers,
407-
);
408-
if (shouldRemove) {
409-
removedIssues.add(issue);
410-
}
411-
return !shouldRemove;
412-
});
413-
414-
if (removedIssues.size === 0) {
415-
return result;
416-
}
417-
418-
return {
419-
safe: remainingIssues.length === 0,
420-
issues: remainingIssues,
421-
diagnostics: result.diagnostics.filter(
422-
(diagnostic) => !removedIssues.has(diagnostic.message),
423-
),
424-
};
425-
}
426-
427-
async function collectAutoPinnableBareSpecifiers(
428-
plan: RuntimePlan,
429-
): Promise<Set<string>> {
430-
const specifiers = new Set<string>();
431-
432-
for (const specifier of plan.imports ?? []) {
433-
addAutoPinnableBareSpecifier(specifiers, specifier);
434-
}
435-
436-
for (const specifier of plan.capabilities?.allowedModules ?? []) {
437-
addAutoPinnableBareSpecifier(specifiers, specifier);
438-
}
439-
440-
walkPlanNodes(plan.root, (node) => {
441-
if (node.type === "component") {
442-
addAutoPinnableBareSpecifier(specifiers, node.module);
443-
}
444-
});
445-
446-
for (const specifier of await collectRuntimeSourceImports(
447-
plan.source?.code ?? "",
448-
)) {
449-
addAutoPinnableBareSpecifier(specifiers, specifier);
450-
}
451-
452-
return specifiers;
453-
}
454-
455-
function walkPlanNodes(
456-
node: RuntimePlan["root"],
457-
visitor: (node: RuntimePlan["root"]) => void,
458-
): void {
459-
visitor(node);
460-
if (node.type === "text") {
461-
return;
462-
}
463-
464-
for (const child of node.children ?? []) {
465-
walkPlanNodes(child, visitor);
366+
const policyPrecheckResult = await precheckSecurity.checkPlan(plan);
367+
if (!policyPrecheckResult.safe) {
368+
return policyPrecheckResult;
466369
}
467-
}
468370

469-
function addAutoPinnableBareSpecifier(
470-
target: Set<string>,
471-
specifier: string,
472-
): void {
473-
if (isAutoPinnableBareSpecifier(specifier)) {
474-
target.add(specifier);
475-
}
476-
}
477-
478-
function isAutoPinnableBareSpecifier(specifier: string): boolean {
479-
const normalized = specifier.trim();
480-
if (normalized.length === 0) {
481-
return false;
371+
if (callerProvidedSecurity && !isPlainDefaultSecurityChecker(security)) {
372+
return security.checkPlan(plan);
482373
}
483374

484-
const lowered = normalized.toLowerCase();
485-
return (
486-
!normalized.startsWith("./") &&
487-
!normalized.startsWith("../") &&
488-
!normalized.startsWith("/") &&
489-
!lowered.startsWith("http://") &&
490-
!lowered.startsWith("https://") &&
491-
!lowered.startsWith("data:") &&
492-
!lowered.startsWith("blob:") &&
493-
!lowered.startsWith("inline://") &&
494-
lowered !== "this-plan-source"
495-
);
375+
return policyPrecheckResult;
496376
}
497377

498-
function isAutoPinnablePrecheckIssue(
499-
issue: string,
500-
autoPinnableSpecifiers: ReadonlySet<string>,
501-
): boolean {
378+
function isPlainDefaultSecurityChecker(
379+
security: NonNullable<RuntimeEmbedRenderOptions["security"]>,
380+
): security is DefaultSecurityChecker {
502381
return (
503-
matchesAutoPinnableSpecifierIssue(
504-
issue,
505-
"Missing moduleManifest entry for bare specifier: ",
506-
autoPinnableSpecifiers,
507-
) ||
508-
matchesAutoPinnableSpecifierIssue(
509-
issue,
510-
"Runtime source bare import requires manifest entry: ",
511-
autoPinnableSpecifiers,
512-
) ||
513-
matchesAutoPinnableSpecifierIssue(
514-
issue,
515-
"Module specifier is not in allowlist: ",
516-
autoPinnableSpecifiers,
517-
)
382+
security instanceof DefaultSecurityChecker &&
383+
security.constructor === DefaultSecurityChecker
518384
);
519385
}
520386

521-
function matchesAutoPinnableSpecifierIssue(
522-
issue: string,
523-
prefix: string,
524-
autoPinnableSpecifiers: ReadonlySet<string>,
525-
): boolean {
526-
if (!issue.startsWith(prefix)) {
527-
return false;
528-
}
529-
530-
const specifier = issue.slice(prefix.length).trim();
531-
return autoPinnableSpecifiers.has(specifier);
532-
}
533-
534387
function mergeSecurityInitializationProfile(
535388
input: RuntimeEmbedRenderOptions["securityInitialization"],
536389
profile: RuntimeSecurityProfile,

tests/module-manifest-autopin.test.ts

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,110 @@ test("renderPlanInBrowser honors custom security rejection before auto-pin fetch
368368
}
369369
});
370370

371+
test("renderPlanInBrowser preserves caller checker rejections that match auto-pin issue text", async () => {
372+
const baseChecker = new DefaultSecurityChecker();
373+
baseChecker.initialize({ profile: "balanced" });
374+
375+
const customChecker = {
376+
initialize(): void {},
377+
getPolicy() {
378+
return baseChecker.getPolicy();
379+
},
380+
getProfile() {
381+
return baseChecker.getProfile();
382+
},
383+
checkCapabilities(
384+
...args: Parameters<DefaultSecurityChecker["checkCapabilities"]>
385+
) {
386+
return baseChecker.checkCapabilities(...args);
387+
},
388+
checkModuleSpecifier(specifier: string) {
389+
return baseChecker.checkModuleSpecifier(specifier);
390+
},
391+
async checkPlan() {
392+
return {
393+
safe: false,
394+
issues: ["Module specifier is not in allowlist: date-fns"],
395+
diagnostics: [
396+
{
397+
level: "error" as const,
398+
code: "CUSTOM_CHECKER_REJECTED",
399+
message: "Module specifier is not in allowlist: date-fns",
400+
},
401+
],
402+
};
403+
},
404+
};
405+
406+
const plan: RuntimePlan = {
407+
specVersion: DEFAULT_RUNTIME_PLAN_SPEC_VERSION,
408+
id: "autopin_embed_custom_checker_issue_collision_plan",
409+
version: 1,
410+
root: createElementNode("section", undefined, [createTextNode("fallback")]),
411+
capabilities: {
412+
domWrite: true,
413+
},
414+
source: {
415+
language: "js",
416+
runtime: "renderify",
417+
code: [
418+
'import { format } from "date-fns";',
419+
"export default function App(){",
420+
" return { type: 'text', value: format(new Date(0), 'yyyy-MM-dd') };",
421+
"}",
422+
].join("\n"),
423+
},
424+
};
425+
426+
let fetchCount = 0;
427+
const originalFetch = globalThis.fetch;
428+
globalThis.fetch = (async () => {
429+
fetchCount += 1;
430+
return new Response("unexpected network", { status: 500 });
431+
}) as typeof fetch;
432+
433+
try {
434+
await assert.rejects(
435+
() =>
436+
renderPlanInBrowser(plan, {
437+
security: customChecker,
438+
runtime: {
439+
async initialize(): Promise<void> {},
440+
async terminate(): Promise<void> {},
441+
async probePlan(): Promise<never> {
442+
throw new Error("not implemented");
443+
},
444+
async executePlan(): Promise<never> {
445+
throw new Error("not implemented");
446+
},
447+
async execute(): Promise<never> {
448+
throw new Error("runtime should not execute");
449+
},
450+
async compile(): Promise<string> {
451+
return "";
452+
},
453+
getPlanState() {
454+
return undefined;
455+
},
456+
setPlanState(): void {},
457+
clearPlanState(): void {},
458+
} as unknown as RuntimeManager,
459+
autoPinModuleLoader: new ResolveOnlyJspmLoader(),
460+
autoInitializeRuntime: false,
461+
autoTerminateRuntime: false,
462+
}),
463+
(error: unknown) =>
464+
error instanceof RuntimeSecurityViolationError &&
465+
error.result.issues.includes(
466+
"Module specifier is not in allowlist: date-fns",
467+
),
468+
);
469+
assert.equal(fetchCount, 0);
470+
} finally {
471+
globalThis.fetch = originalFetch;
472+
}
473+
});
474+
371475
test("renderPlanInBrowser auto-pins bare imports after security precheck", async () => {
372476
const plan: RuntimePlan = {
373477
specVersion: DEFAULT_RUNTIME_PLAN_SPEC_VERSION,

0 commit comments

Comments
 (0)