Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
50 changes: 44 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,46 @@ 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) &&
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 +330,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
22 changes: 22 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,28 @@ 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("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