Skip to content

Commit 8191d3f

Browse files
timokoesslertimokoessler
committed
Support express instrumentation
- Allow modifying arguments object directly - Add sample app
1 parent 7c14449 commit 8191d3f

File tree

14 files changed

+1270
-6
lines changed

14 files changed

+1270
-6
lines changed

‎instrumentation-wasm/src/js_transformer/helpers/get_name_str_for_member_expr.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ pub fn get_name_str_for_member_expr<'a>(
55
allocator: &'a Allocator,
66
member_expr: &MemberExpression,
77
) -> Option<&'a str> {
8-
let mut obj_name_str: &str = "";
9-
let mut prop_name_str: &str = "";
8+
let obj_name_str: &str;
9+
let prop_name_str: &str;
1010

1111
match member_expr {
1212
MemberExpression::StaticMemberExpression(static_member_expr) => {

‎instrumentation-wasm/src/js_transformer/helpers/insert_code.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,24 @@ pub fn insert_modify_args<'a>(
4242
identifier: &str,
4343
arg_names_str: &str,
4444
body: &mut Box<'a, FunctionBody<'a>>,
45+
modify_arguments_object: bool,
4546
) {
47+
if modify_arguments_object {
48+
// If we are modifying the arguments object, we need to use the arguments object directly
49+
// instead of the individual arguments
50+
let source_text: &'a str = allocator.alloc_str(&format!(
51+
"Object.assign(arguments, __instrumentModifyArgs('{}', Array.from(arguments)));",
52+
identifier
53+
));
54+
insert_single_statement_into_func(allocator, body, 0, source_text);
55+
return;
56+
}
57+
58+
if arg_names_str.is_empty() {
59+
// If there are no arguments to modify, we can skip this step
60+
return;
61+
}
62+
4663
let source_text: &'a str = allocator.alloc_str(&format!(
4764
"[{}] = __instrumentModifyArgs('{}', [{}]);",
4865
arg_names_str, identifier, arg_names_str

‎instrumentation-wasm/src/js_transformer/instructions.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,5 @@ pub struct FunctionInstructions {
1717
pub inspect_args: bool,
1818
pub modify_args: bool,
1919
pub modify_return_value: bool,
20+
pub modify_arguments_object: bool,
2021
}

‎instrumentation-wasm/src/js_transformer/transformer.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use oxc_allocator::Allocator;
22
use oxc_codegen::{Codegen, CodegenOptions};
3-
use oxc_parser::{ParseOptions, Parser};
3+
use oxc_parser::Parser;
44
use oxc_semantic::SemanticBuilder;
55
use oxc_span::SourceType;
66
use oxc_traverse::traverse_mut;

‎instrumentation-wasm/src/js_transformer/transformer_impl.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ impl<'a> Traverse<'a> for Transformer<'a> {
6262
&instruction.identifier,
6363
&arg_names_str,
6464
body,
65+
instruction.modify_arguments_object,
6566
);
6667
}
6768

@@ -130,13 +131,14 @@ impl<'a> Traverse<'a> for Transformer<'a> {
130131

131132
let body = function_expression.body.as_mut().unwrap();
132133

133-
if instruction.modify_args && !arg_names.is_empty() {
134+
if instruction.modify_args {
134135
let arg_names_str = arg_names.join(", ");
135136
insert_modify_args(
136137
self.allocator,
137138
&instruction.identifier,
138139
&arg_names_str,
139140
body,
141+
instruction.modify_arguments_object,
140142
);
141143
}
142144

‎library/agent/hooks/instrumentation/codeTransformation.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ t.test("add inspectArgs to method definition (ESM)", async (t) => {
3636
inspectArgs: true,
3737
modifyArgs: false,
3838
modifyReturnValue: false,
39+
modifyArgumentsObject: false,
3940
},
4041
],
4142
}
@@ -93,6 +94,7 @@ t.test("add inspectArgs to method definition (CJS)", async (t) => {
9394
inspectArgs: true,
9495
modifyArgs: false,
9596
modifyReturnValue: false,
97+
modifyArgumentsObject: false,
9698
},
9799
],
98100
}
@@ -150,6 +152,7 @@ t.test("wrong function name", async (t) => {
150152
inspectArgs: true,
151153
modifyArgs: false,
152154
modifyReturnValue: false,
155+
modifyArgumentsObject: false,
153156
},
154157
],
155158
}
@@ -206,6 +209,7 @@ t.test("typescript code", async (t) => {
206209
inspectArgs: true,
207210
modifyArgs: false,
208211
modifyReturnValue: false,
212+
modifyArgumentsObject: false,
209213
},
210214
],
211215
}
@@ -263,6 +267,7 @@ t.test("typescript code in a js file", async (t) => {
263267
inspectArgs: true,
264268
modifyArgs: false,
265269
modifyReturnValue: false,
270+
modifyArgumentsObject: false,
266271
},
267272
],
268273
}
@@ -292,6 +297,7 @@ t.test("empty code", async (t) => {
292297
inspectArgs: true,
293298
modifyArgs: false,
294299
modifyReturnValue: false,
300+
modifyArgumentsObject: false,
295301
},
296302
],
297303
});
@@ -336,6 +342,7 @@ t.test("add modifyArgs to method definition (ESM)", async (t) => {
336342
inspectArgs: false,
337343
modifyArgs: true,
338344
modifyReturnValue: false,
345+
modifyArgumentsObject: false,
339346
},
340347
],
341348
}
@@ -395,6 +402,7 @@ t.test(
395402
inspectArgs: true,
396403
modifyArgs: true,
397404
modifyReturnValue: false,
405+
modifyArgumentsObject: false,
398406
},
399407
],
400408
}
@@ -449,6 +457,7 @@ t.test("modify rest parameter args", async (t) => {
449457
inspectArgs: false,
450458
modifyArgs: true,
451459
modifyReturnValue: false,
460+
modifyArgumentsObject: false,
452461
},
453462
],
454463
}
@@ -494,6 +503,7 @@ t.test("modify rest parameter args", async (t) => {
494503
inspectArgs: false,
495504
modifyArgs: true,
496505
modifyReturnValue: false,
506+
modifyArgumentsObject: false,
497507
},
498508
],
499509
}
@@ -548,6 +558,7 @@ t.test("add inspectArgs to method definition (unambiguous)", async (t) => {
548558
inspectArgs: true,
549559
modifyArgs: false,
550560
modifyReturnValue: false,
561+
modifyArgumentsObject: false,
551562
},
552563
],
553564
}
@@ -603,6 +614,7 @@ t.test("add inspectArgs to method definition (unambiguous)", async (t) => {
603614
inspectArgs: true,
604615
modifyArgs: false,
605616
modifyReturnValue: false,
617+
modifyArgumentsObject: false,
606618
},
607619
],
608620
}
@@ -654,6 +666,7 @@ t.test(
654666
inspectArgs: true,
655667
modifyArgs: false,
656668
modifyReturnValue: false,
669+
modifyArgumentsObject: false,
657670
},
658671
],
659672
}
@@ -699,6 +712,7 @@ t.test(
699712
inspectArgs: false,
700713
modifyArgs: true,
701714
modifyReturnValue: false,
715+
modifyArgumentsObject: false,
702716
},
703717
],
704718
}
@@ -745,6 +759,7 @@ t.test(
745759
inspectArgs: true,
746760
modifyArgs: false,
747761
modifyReturnValue: false,
762+
modifyArgumentsObject: false,
748763
},
749764
],
750765
}
@@ -792,6 +807,7 @@ t.test(
792807
inspectArgs: false,
793808
modifyArgs: true,
794809
modifyReturnValue: false,
810+
modifyArgumentsObject: false,
795811
},
796812
],
797813
}
@@ -813,6 +829,54 @@ t.test(
813829
}
814830
);
815831

832+
t.test(
833+
"add modifyArgs to dynamic function assignment expression with arguments object (CJS)",
834+
async (t) => {
835+
const result = transformCode(
836+
"express",
837+
"1.0.0",
838+
"application.js",
839+
`
840+
const app = require("example");
841+
const key = "get";
842+
app[key] = function (fn) {
843+
console.log("test");
844+
};
845+
`,
846+
"commonjs",
847+
{
848+
path: "application.js",
849+
versionRange: "^1.0.0",
850+
functions: [
851+
{
852+
nodeType: "FunctionAssignment",
853+
name: "app[key]",
854+
identifier: "express.application.js.app[key].v1.0.0",
855+
inspectArgs: false,
856+
modifyArgs: true,
857+
modifyReturnValue: false,
858+
modifyArgumentsObject: true,
859+
},
860+
],
861+
}
862+
);
863+
864+
t.same(
865+
compareCodeStrings(
866+
result,
867+
`const { __instrumentInspectArgs, __instrumentModifyArgs } = require("@aikidosec/firewall/instrument/internals");
868+
const app = require("example");
869+
const key = "get";
870+
app[key] = function (fn) {
871+
Object.assign(arguments, __instrumentModifyArgs("express.application.js.app[key].v1.0.0", Array.from(arguments)));
872+
console.log("test");
873+
};`
874+
),
875+
true
876+
);
877+
}
878+
);
879+
816880
t.test("does not modify code if function name is not found", async (t) => {
817881
const result = transformCode(
818882
"express",
@@ -837,6 +901,7 @@ t.test("does not modify code if function name is not found", async (t) => {
837901
inspectArgs: false,
838902
modifyArgs: true,
839903
modifyReturnValue: false,
904+
modifyArgumentsObject: false,
840905
},
841906
],
842907
}

‎library/agent/hooks/instrumentation/instructions.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ t.test("it works", async (t) => {
5858
inspectArgs: true,
5959
modifyArgs: false,
6060
modifyReturnValue: false,
61+
modifyArgumentsObject: false,
6162
},
6263
],
6364
});

‎library/agent/hooks/instrumentation/instructions.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export function setPackagesToInstrument(_packages: Package[]) {
4848
inspectArgs: !!func.inspectArgs,
4949
modifyArgs: !!func.modifyArgs,
5050
modifyReturnValue: !!func.modifyReturnValue,
51+
modifyArgumentsObject: func.modifyArgumentsObject ?? false,
5152
};
5253
}),
5354
};

‎library/agent/hooks/instrumentation/types.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,22 @@ export type IntereptorFunctionsObj = {
6060
};
6161

6262
export type PackageFunctionInstrumentationInstruction = {
63-
nodeType: "MethodDefinition";
63+
nodeType: "MethodDefinition" | "FunctionAssignment";
6464
name: string;
6565
inspectArgs?: InspectArgsInterceptor;
6666
modifyArgs?: ModifyArgsInterceptor;
6767
modifyReturnValue?: ModifyReturnValueInterceptor;
68+
/**
69+
* If true, the arguments object will be modified by modifyArgs instead modifying the named arguments.
70+
*
71+
* Why is this needed?
72+
* If the library object uses the arguments object instead of named function arguments and no named arguments are defined in the function / method definition
73+
* or not all arguments are defined, we need to modify the arguments object instead of the named arguments.
74+
*
75+
* Important to know:
76+
* In strict mode, the arguments object and the named arguments are not synced, so changing the arguments object will not change the named arguments and vice versa.
77+
*/
78+
modifyArgumentsObject?: boolean;
6879
};
6980

7081
export type PackageFileInstrumentationInstruction = {
@@ -76,11 +87,12 @@ export type PackageFileInstrumentationInstructionJSON = {
7687
path: string; // Relative path to required file inside the package folder
7788
versionRange: string;
7889
functions: {
79-
nodeType: "MethodDefinition" | "FunctionAssignment";
90+
nodeType: PackageFunctionInstrumentationInstruction["nodeType"];
8091
name: string;
8192
identifier: string;
8293
inspectArgs: boolean;
8394
modifyArgs: boolean;
8495
modifyReturnValue: boolean;
96+
modifyArgumentsObject: boolean;
8597
}[];
8698
};

‎library/sources/Express.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,23 @@ export class Express implements Wrapper {
4848
wrapExport(exports.application, "use", pkgInfo, {
4949
modifyArgs: (args) => this.wrapArgs(args),
5050
});
51+
})
52+
.addFileInstrumentation({
53+
path: "lib/application.js",
54+
functions: [
55+
{
56+
nodeType: "FunctionAssignment",
57+
name: "app.use",
58+
modifyArgumentsObject: true,
59+
modifyArgs: (args) => this.wrapArgs(args),
60+
},
61+
{
62+
nodeType: "FunctionAssignment",
63+
name: "app[method]",
64+
modifyArgumentsObject: true,
65+
modifyArgs: (args) => this.wrapArgs(args),
66+
},
67+
],
5168
});
5269
}
5370
}

0 commit comments

Comments
 (0)