Skip to content

Commit f66af80

Browse files
authored
refactor: move Signals to the probe.main context (#396)
1 parent fad019f commit f66af80

File tree

9 files changed

+63
-50
lines changed

9 files changed

+63
-50
lines changed

.changeset/pretty-moose-go.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@nodesecure/js-x-ray": minor
3+
---
4+
5+
Move Signals into probe.main context

workspaces/js-x-ray/docs/AstAnalyser.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ export const customProbes = [
147147
node.type === "VariableDeclaration" && node.declarations[0].init.value === "danger"
148148
],
149149
main: (node, ctx) => {
150-
const { sourceFile, data: calleeName } = ctx;
150+
const { sourceFile, data: calleeName, signals } = ctx;
151151
if (node.declarations[0].init.value === "danger") {
152152
sourceFile.warnings.push({
153153
kind: "unsafe-danger",
@@ -158,7 +158,7 @@ export const customProbes = [
158158
severity: "Warning"
159159
});
160160

161-
return ProbeSignals.Skip;
161+
return signals.Skip;
162162
}
163163

164164
return null;

workspaces/js-x-ray/src/ProbeRunner.ts

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ export type ProbeContext<T extends ProbeContextDef = ProbeContextDef> = {
2929
sourceFile: SourceFile;
3030
context?: T;
3131
};
32+
export type ProbeMainContext<T extends ProbeContextDef = ProbeContextDef> = ProbeContext<T> & {
33+
data?: any;
34+
signals: typeof ProbeRunner.Signals;
35+
};
3236

3337
export type ProbeValidationCallback<T extends ProbeContextDef = ProbeContextDef> = (
3438
node: ESTree.Node, ctx: ProbeContext<T>
@@ -41,23 +45,24 @@ export interface Probe<T extends ProbeContextDef = ProbeContextDef> {
4145
validateNode: ProbeValidationCallback<T> | ProbeValidationCallback<T>[];
4246
main: (
4347
node: any,
44-
ctx: ProbeContext<T> & { data?: any; }
48+
ctx: ProbeMainContext<T>
4549
) => ProbeReturn;
4650
teardown?: (ctx: ProbeContext<T>) => void;
4751
breakOnMatch?: boolean;
4852
breakGroup?: string;
4953
context?: ProbeContext<T>;
5054
}
5155

52-
export const ProbeSignals = Object.freeze({
53-
Break: Symbol.for("breakWalk"),
54-
Skip: Symbol.for("skipWalk")
55-
});
56-
5756
export class ProbeRunner {
5857
probes: Probe[];
5958
sourceFile: SourceFile;
6059

60+
static Signals = Object.freeze({
61+
Break: Symbol.for("breakWalk"),
62+
Skip: Symbol.for("skipWalk"),
63+
Continue: null
64+
});
65+
6166
/**
6267
* Note:
6368
* The order of the table has an importance/impact on the correct execution of the probes
@@ -138,6 +143,7 @@ export class ProbeRunner {
138143
if (isMatching) {
139144
return probe.main(node, {
140145
...ctx,
146+
signals: ProbeRunner.Signals,
141147
data
142148
});
143149
}
@@ -157,15 +163,15 @@ export class ProbeRunner {
157163
}
158164

159165
try {
160-
const result = this.#runProbe(probe, node);
161-
if (result === null) {
166+
const signal = this.#runProbe(probe, node);
167+
if (signal === ProbeRunner.Signals.Continue) {
162168
continue;
163169
}
164170

165-
if (result === ProbeSignals.Skip) {
171+
if (signal === ProbeRunner.Signals.Skip) {
166172
return "skip";
167173
}
168-
if (result === ProbeSignals.Break || probe.breakOnMatch) {
174+
if (signal === ProbeRunner.Signals.Break || probe.breakOnMatch) {
169175
const breakGroup = probe.breakGroup || null;
170176

171177
if (breakGroup === null) {

workspaces/js-x-ray/src/probes/isRequire/isRequire.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
import type { ESTree } from "meriyah";
1111

1212
// Import Internal Dependencies
13-
import { ProbeSignals, type ProbeContext } from "../../ProbeRunner.js";
13+
import type { ProbeContext, ProbeMainContext } from "../../ProbeRunner.js";
1414
import { isLiteral } from "../../types/estree.js";
1515
import { RequireCallExpressionWalker } from "./RequireCallExpressionWalker.js";
1616
import { generateWarning } from "../../warnings.js";
@@ -70,9 +70,9 @@ function teardown(
7070

7171
function main(
7272
node: ESTree.CallExpression,
73-
options: ProbeContext & { data?: string; }
73+
ctx: ProbeMainContext
7474
) {
75-
const { sourceFile, data: calleeName } = options;
75+
const { sourceFile, data: calleeName, signals } = ctx;
7676
const { tracer } = sourceFile;
7777

7878
if (node.arguments.length === 0) {
@@ -170,7 +170,7 @@ function main(
170170
}
171171

172172
// We skip walking the tree to avoid anymore warnings...
173-
return ProbeSignals.Skip;
173+
return signals.Skip;
174174
}
175175

176176
default:

workspaces/js-x-ray/src/probes/isSerializeEnv.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ import type { ESTree } from "meriyah";
77

88
// Import Internal Dependencies
99
import { generateWarning } from "../warnings.js";
10-
import { ProbeSignals, type ProbeContext } from "../ProbeRunner.js";
10+
import type {
11+
ProbeContext,
12+
ProbeMainContext
13+
} from "../ProbeRunner.js";
1114

1215
/**
1316
* @description Detect serialization of process.env which could indicate environment variable exfiltration
@@ -59,17 +62,17 @@ function validateNode(
5962

6063
function main(
6164
node: ESTree.Node,
62-
ctx: ProbeContext
65+
ctx: ProbeMainContext
6366
) {
64-
const { sourceFile } = ctx;
67+
const { sourceFile, signals } = ctx;
6568

6669
const warning = generateWarning("serialize-environment", {
6770
value: "JSON.stringify(process.env)",
6871
location: node.loc
6972
});
7073
sourceFile.warnings.push(warning);
7174

72-
return ProbeSignals.Skip;
75+
return signals.Skip;
7376
}
7477

7578
function initialize(

workspaces/js-x-ray/src/probes/isUnsafeCallee.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,8 @@ import type { ESTree } from "meriyah";
33
import { getCallExpressionIdentifier } from "@nodesecure/estree-ast-utils";
44

55
// Import Internal Dependencies
6-
import { SourceFile } from "../SourceFile.js";
76
import { generateWarning } from "../warnings.js";
8-
import { ProbeSignals } from "../ProbeRunner.js";
7+
import type { ProbeMainContext } from "../ProbeRunner.js";
98

109
/**
1110
* @description Detect unsafe statement
@@ -21,19 +20,19 @@ function validateNode(
2120

2221
function main(
2322
node: ESTree.CallExpression,
24-
options: { sourceFile: SourceFile; data?: string; }
23+
ctx: ProbeMainContext
2524
) {
26-
const { sourceFile, data: calleeName } = options;
25+
const { sourceFile, data: calleeName, signals } = ctx;
2726

2827
if (!calleeName) {
29-
return ProbeSignals.Skip;
28+
return signals.Skip;
3029
}
3130
if (
3231
calleeName === "Function" &&
3332
node.callee.arguments.length > 0 &&
3433
node.callee.arguments[0].value === "return this"
3534
) {
36-
return ProbeSignals.Skip;
35+
return signals.Skip;
3736
}
3837

3938
const warning = generateWarning("unsafe-stmt", {
@@ -42,7 +41,7 @@ function main(
4241
});
4342
sourceFile.warnings.push(warning);
4443

45-
return ProbeSignals.Skip;
44+
return signals.Skip;
4645
}
4746

4847
function isEvalCallee(

workspaces/js-x-ray/src/probes/isUnsafeCommand.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
import type { ESTree } from "meriyah";
33

44
// Import Internal Dependencies
5-
import { SourceFile } from "../SourceFile.js";
65
import { generateWarning } from "../warnings.js";
7-
import { ProbeSignals } from "../ProbeRunner.js";
8-
import { isLiteral, isTemplateLiteral } from "../types/estree.js";
6+
import {
7+
isLiteral,
8+
isTemplateLiteral
9+
} from "../types/estree.js";
10+
import type { ProbeMainContext } from "../ProbeRunner.js";
911

1012
// CONSTANTS
1113
const kUnsafeCommands = ["csrutil", "uname", "ping", "curl"];
@@ -102,9 +104,9 @@ function validateNode(
102104

103105
function main(
104106
node: ESTree.CallExpression,
105-
options: { sourceFile: SourceFile; data?: string; }
107+
ctx: ProbeMainContext
106108
) {
107-
const { sourceFile, data: methodName } = options;
109+
const { sourceFile, data: methodName, signals } = ctx;
108110

109111
const commandArg = node.arguments[0];
110112
if (!isLiteral(commandArg) && !isTemplateLiteral(commandArg)) {
@@ -134,7 +136,7 @@ function main(
134136
});
135137
sourceFile.warnings.push(warning);
136138

137-
return ProbeSignals.Skip;
139+
return signals.Skip;
138140
}
139141

140142
return null;

workspaces/js-x-ray/test/ProbeRunner.spec.ts

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ import type { ESTree } from "meriyah";
77

88
// Import Internal Dependencies
99
import {
10-
ProbeRunner,
11-
ProbeSignals
10+
ProbeRunner
1211
} from "../src/ProbeRunner.js";
1312
import { SourceFile } from "../src/SourceFile.js";
1413

@@ -113,7 +112,7 @@ describe("ProbeRunner", () => {
113112

114113
assert.strictEqual(fakeProbe.main.mock.calls.length, 1);
115114
assert.deepEqual(fakeProbe.main.mock.calls.at(0)?.arguments, [
116-
astNode, { sourceFile, data: null, context: undefined }
115+
astNode, { sourceFile, data: null, context: undefined, signals: ProbeRunner.Signals }
117116
]);
118117

119118
assert.strictEqual(fakeProbe.teardown.mock.calls.length, 1);
@@ -126,7 +125,7 @@ describe("ProbeRunner", () => {
126125
const data = { test: "data" };
127126
const fakeProbe = {
128127
validateNode: mock.fn((_: ESTree.Node) => [true, data]),
129-
main: mock.fn(() => ProbeSignals.Skip)
128+
main: mock.fn(() => ProbeRunner.Signals.Skip)
130129
};
131130

132131
const sourceFile = new SourceFile();
@@ -149,14 +148,14 @@ describe("ProbeRunner", () => {
149148
astNode, expectedContext
150149
]);
151150
assert.deepEqual(fakeProbe.main.mock.calls.at(0)?.arguments, [
152-
astNode, { ...expectedContext, data }
151+
astNode, { ...expectedContext, data, signals: ProbeRunner.Signals }
153152
]);
154153
});
155154

156155
it("should trigger and return a skip signal", () => {
157156
const fakeProbe = {
158157
validateNode: (node: ESTree.Node) => [node.type === "Literal"],
159-
main: () => ProbeSignals.Skip,
158+
main: () => ProbeRunner.Signals.Skip,
160159
teardown: mock.fn()
161160
};
162161

@@ -181,20 +180,20 @@ describe("ProbeRunner", () => {
181180
it("should call the finalize methods", () => {
182181
const fakeProbe = {
183182
validateNode: (_: ESTree.Node) => [true],
184-
main: () => ProbeSignals.Skip,
183+
main: () => ProbeRunner.Signals.Skip,
185184
finalize: mock.fn()
186185
};
187186

188187
const fakeProbeSkip = {
189188
validateNode: (_: ESTree.Node) => [true],
190-
main: () => ProbeSignals.Skip,
189+
main: () => ProbeRunner.Signals.Skip,
191190
teardown: mock.fn(),
192191
finalize: mock.fn()
193192
};
194193

195194
const fakeProbeBreak = {
196195
validateNode: (_: ESTree.Node) => [true],
197-
main: () => ProbeSignals.Break,
196+
main: () => ProbeRunner.Signals.Break,
198197
teardown: mock.fn(),
199198
finalize: mock.fn()
200199
};
@@ -227,7 +226,7 @@ describe("ProbeRunner", () => {
227226
const fakeProbe = {
228227
initialize: mock.fn(() => fakeCtx),
229228
validateNode: mock.fn((_: ESTree.Node) => [true]),
230-
main: mock.fn(() => ProbeSignals.Skip),
229+
main: mock.fn(() => ProbeRunner.Signals.Skip),
231230
finalize: mock.fn()
232231
};
233232

@@ -251,7 +250,7 @@ describe("ProbeRunner", () => {
251250
astNode, expectedContext
252251
]);
253252
assert.deepEqual(fakeProbe.main.mock.calls.at(0)?.arguments, [
254-
astNode, { ...expectedContext, data: null }
253+
astNode, { ...expectedContext, data: null, signals: ProbeRunner.Signals }
255254
]);
256255
assert.deepEqual(fakeProbe.initialize.mock.calls.at(0)?.arguments, [
257256
{ sourceFile, context: undefined }
@@ -266,7 +265,7 @@ describe("ProbeRunner", () => {
266265
const fakeProbe = {
267266
initialize: mock.fn(),
268267
validateNode: mock.fn((_: ESTree.Node) => [true]),
269-
main: mock.fn(() => ProbeSignals.Skip),
268+
main: mock.fn(() => ProbeRunner.Signals.Skip),
270269
finalize: mock.fn(),
271270
context: fakeCtx
272271
};
@@ -291,7 +290,7 @@ describe("ProbeRunner", () => {
291290
astNode, expectedContext
292291
]);
293292
assert.deepEqual(fakeProbe.main.mock.calls.at(0)?.arguments, [
294-
astNode, { ...expectedContext, data: null }
293+
astNode, { ...expectedContext, data: null, signals: ProbeRunner.Signals }
295294
]);
296295
assert.deepEqual(fakeProbe.finalize.mock.calls.at(0)?.arguments, [
297296
expectedContext

workspaces/js-x-ray/test/utils/index.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {
1010
} from "../../src/index.js";
1111
import {
1212
ProbeRunner,
13-
ProbeSignals,
1413
type Probe
1514
} from "../../src/ProbeRunner.js";
1615

@@ -82,8 +81,8 @@ export const customProbes: Probe[] = [
8281
node.declarations[0].init.value === "danger"
8382
];
8483
},
85-
main(node, options) {
86-
const { sourceFile, data: calleeName } = options;
84+
main(node, ctx) {
85+
const { sourceFile, data: calleeName, signals } = ctx;
8786
if (node.declarations[0].init.value === "danger") {
8887
sourceFile.warnings.push({
8988
kind: "unsafe-danger",
@@ -94,7 +93,7 @@ export const customProbes: Probe[] = [
9493
severity: "Warning"
9594
});
9695

97-
return ProbeSignals.Skip;
96+
return signals.Skip;
9897
}
9998

10099
return null;

0 commit comments

Comments
 (0)