Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions .github/actions/ci-static-checks/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ runs:
shell: bash
run: npm install --ignore-scripts

- name: Enforce base-trusted createRequire allowlist ratchet
shell: bash
run: npx tsx "$GITHUB_ACTION_PATH/create-require-ratchet.mts"

- name: Validate config schemas
shell: bash
run: npm run validate:configs
Expand Down
51 changes: 45 additions & 6 deletions scripts/checks/openshell-policy-mutation-read.mts
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,17 @@ function isPolicyBuilderModule(
);
}

function requireModuleSpecifier(expression: ts.Expression | undefined): string | null {
function requireModuleSpecifier(
expression: ts.Expression | undefined,
checker: ts.TypeChecker,
): string | null {
if (
!expression ||
!ts.isCallExpression(expression) ||
!ts.isIdentifier(expression.expression) ||
expression.expression.text !== "require" ||
expression.arguments.length !== 1
expression.arguments.length !== 1 ||
checker.getSymbolAtLocation(expression.expression)
) {
return null;
}
Expand All @@ -143,7 +147,7 @@ function collectRequiredPolicyBindings(
identifiers: Set<ts.Symbol>,
namespaces: Set<ts.Symbol>,
): void {
const moduleSpecifier = requireModuleSpecifier(declaration.initializer);
const moduleSpecifier = requireModuleSpecifier(declaration.initializer, checker);
if (!moduleSpecifier || !isPolicyBuilderModule(fileName, moduleSpecifier, repoRoot)) return;
if (ts.isIdentifier(declaration.name)) {
const symbol = checker.getSymbolAtLocation(declaration.name);
Expand Down Expand Up @@ -258,15 +262,47 @@ function literalText(expression: ts.Expression): string | null {
return ts.isStringLiteralLike(expression) ? expression.text : null;
}

function isDirectPolicyRead(expression: ts.ArrayLiteralExpression): boolean {
function isCanonicalOpenshellResolverCall(
expression: ts.Expression,
fileName: string,
repoRoot: string,
checker: ts.TypeChecker,
): boolean {
if (
!ts.isCallExpression(expression) ||
!ts.isIdentifier(expression.expression) ||
expression.expression.text !== "resolveOpenshellBinary" ||
expression.arguments.length !== 0 ||
path.resolve(fileName) !== path.resolve(repoRoot, "src/lib/policy/commands.ts")
) {
return false;
}
const symbol = checker.getSymbolAtLocation(expression.expression);
return (
symbol?.declarations?.some(
(declaration) =>
ts.isFunctionDeclaration(declaration) &&
ts.isSourceFile(declaration.parent) &&
declaration.name?.text === "resolveOpenshellBinary" &&
path.resolve(declaration.getSourceFile().fileName) === path.resolve(fileName),
) === true
);
}

function isDirectPolicyRead(
expression: ts.ArrayLiteralExpression,
fileName: string,
repoRoot: string,
checker: ts.TypeChecker,
): boolean {
const first = expression.elements[0];
if (!first || !ts.isExpression(first)) return false;
const firstText = literalText(first);
const offset =
firstText === "policy"
? 0
: firstText === "openshell" ||
(ts.isCallExpression(first) && calledName(first.expression) === "resolveOpenshellBinary")
isCanonicalOpenshellResolverCall(first, fileName, repoRoot, checker)
? 1
: -1;
if (offset < 0) return false;
Expand Down Expand Up @@ -295,7 +331,10 @@ export function countPolicyReadCalls(
isPolicyBuilderCall(node.expression, builderBindings, checker)
) {
readCalls += 1;
} else if (ts.isArrayLiteralExpression(node) && isDirectPolicyRead(node)) {
} else if (
ts.isArrayLiteralExpression(node) &&
isDirectPolicyRead(node, fileName, repoRoot, checker)
) {
readCalls += 1;
}
ts.forEachChild(node, visit);
Expand Down
45 changes: 45 additions & 0 deletions test/policy-mutation-read-discovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,51 @@ describe("OpenShell policy mutation read discovery (#6921)", () => {
expect(countPolicyReadCalls(source, "/repo/src/lib/fixture.ts", "/repo")).toBe(0);
});

it("ignores locally shadowed CommonJS require and OpenShell resolver decoys", () => {
const source = [
"const require = () => ({ buildPolicyGetCommand: () => [] });",
'const { buildPolicyGetCommand } = require("./policy");',
"function resolveOpenshellBinary() { return 'openshell'; }",
"buildPolicyGetCommand(sandboxName);",
'[resolveOpenshellBinary(), "policy", "get", "--base", sandboxName];',
].join("\n");

expect(countPolicyReadCalls(source, "/repo/src/lib/fixture.ts", "/repo")).toBe(0);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

it("ignores a nested resolver shadow in the canonical policy command module", () => {
const source = [
"function resolveOpenshellBinary() { return 'openshell'; }",
"function inspect(resolveOpenshellBinary: () => string) {",
' return [resolveOpenshellBinary(), "policy", "get", "--base", sandboxName];',
"}",
].join("\n");

expect(countPolicyReadCalls(source, "/repo/src/lib/policy/commands.ts", "/repo")).toBe(0);
});

it("ignores a nested resolver function in the canonical policy command module", () => {
const source = [
"function resolveOpenshellBinary() { return 'openshell'; }",
"function inspect() {",
" function resolveOpenshellBinary() { return 'decoy'; }",
' return [resolveOpenshellBinary(), "policy", "get", "--base", sandboxName];',
"}",
].join("\n");

expect(countPolicyReadCalls(source, "/repo/src/lib/policy/commands.ts", "/repo")).toBe(0);
});

it("counts the canonical policy command resolver arrays", () => {
const source = [
"function resolveOpenshellBinary() { return 'openshell'; }",
'const base = [resolveOpenshellBinary(), "policy", "get", "--base", sandboxName];',
'const full = [resolveOpenshellBinary(), "policy", "get", "--full", sandboxName];',
].join("\n");

expect(countPolicyReadCalls(source, "/repo/src/lib/policy/commands.ts", "/repo")).toBe(2);
});

it("ignores a named policy builder import when a nested binding shadows its alias", () => {
const source = [
'import { buildPolicyGetCommand as buildBase } from "./policy/commands";',
Expand Down
3 changes: 3 additions & 0 deletions test/pr-workflow-contract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1074,6 +1074,9 @@ describe("pull request and main workflow contracts", () => {
expect(trustedRatchet.run).toBe(
'node --experimental-strip-types "$GITHUB_ACTION_PATH/create-require-ratchet.mts"',
);
expect(stepRuns(sharedActions.staticChecks)).not.toContain(
'npx tsx "$GITHUB_ACTION_PATH/create-require-ratchet.mts"',
);
expect(
requiredStepIndex(
sharedActions.staticChecks,
Expand Down