Skip to content
Open
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
1 change: 0 additions & 1 deletion .github/workflows/benchmark.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ jobs:
run: |
rustup toolchain install stable
rustup default stable
cargo install wasm-pack
- run: npm install
- run: npm run build
- name: Run NoSQL Injection Benchmark
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/build-and-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ jobs:
run: |
rustup toolchain install stable
rustup default stable
cargo install wasm-pack
- name: Ensure latest npm version
run: npm install -g npm@latest
- name: Install dependencies
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/end-to-end-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ jobs:
run: |
rustup toolchain install stable
rustup default stable
cargo install wasm-pack
- run: npm install
- run: npm run build
- if: matrix.node-version != '24.x'
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/lint-code.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ jobs:
rustup toolchain install stable
rustup default stable
rustup component add rustfmt clippy
cargo install wasm-pack
- run: npm run install-lib-only
- run: npm run build
- run: npm run lint
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/unit-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ jobs:
run: |
rustup toolchain install stable
rustup default stable
cargo install wasm-pack
- name: "Install Google Cloud SDK"
uses: google-github-actions/setup-gcloud@6189d56e4096ee891640bb02ac264be376592d6a # v2
with:
Expand Down
3 changes: 2 additions & 1 deletion instrumentation-wasm/.gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
target/
target/
.bin/
5 changes: 1 addition & 4 deletions instrumentation-wasm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,5 @@ wasm-bindgen = "0.2.101"

[profile.release]
strip = true
opt-level = "s"
opt-level = 3
lto = true

[package.metadata.wasm-pack.profile.release]
wasm-opt = ["-O", "--enable-bulk-memory", "--enable-nontrapping-float-to-int"]
122 changes: 122 additions & 0 deletions instrumentation-wasm/build.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
#!/usr/bin/env pwsh
$ErrorActionPreference = "Stop"

# ------ CONFIGURATION ------
$TARGET = "nodejs"
$OUT_DIR = "..\library\agent\hooks\instrumentation\wasm"
$GENERATE_DTS = $false

$BINARYEN_VERSION = 124
$WASM_BINDGEN_VERSION = "0.2.101"

# ---------------------------

Write-Host "Starting build process for wasm module"

# Always run from script's own directory
$SCRIPT_DIR = Split-Path -Parent (Resolve-Path $MyInvocation.MyCommand.Path)
Push-Location $SCRIPT_DIR

# Create directories
New-Item -ItemType Directory -Path ".\.bin\tmp" -Force | Out-Null
New-Item -ItemType Directory -Path $OUT_DIR -Force | Out-Null

function Download-Binaryen {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate download/extract logic in Download-Binaryen and Download-WasmBindgen should be consolidated into a shared helper to avoid repeated maintenance.

Details

✨ AI Reasoning
​​1) What the code does: Two newly added functions, Download-Binaryen (lines 24-38) and Download-WasmBindgen (lines 40-54), perform nearly identical steps: set ARCH/OS, compose a URL and filename, push into a tmp directory, download an archive, create an extract directory, and extract the archive.
​2) Whether it harms maintainability: This duplication means future fixes (error handling, retry logic, path changes) must be applied in multiple places, increasing risk of divergence and bugs.
​3) Why violation is true/false: The duplication was introduced by this PR (both functions are new and similar), so it is a true, actionable violation that can and should be consolidated into a shared helper to improve maintainability.

🔧 How do I fix it?
Delete extra code. Extract repeated code sequences into reusable functions or methods. Use loops or data structures to eliminate repetitive patterns.

More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.

$BINARYEN_BASE_URL = "https://github.com/WebAssembly/binaryen/releases/download/version_$BINARYEN_VERSION"
$BINARYEN_EXTRACT_DIR = "binaryen"
$ARCH = "x86_64"
$OS = "windows"

$FILE = "binaryen-version_${BINARYEN_VERSION}-${ARCH}-${OS}.tar.gz"

Push-Location ".\.bin\tmp"
Write-Host "Downloading from $BINARYEN_BASE_URL/$FILE"
Invoke-WebRequest "$BINARYEN_BASE_URL/$FILE" -OutFile $FILE
New-Item -ItemType Directory -Path "..\$BINARYEN_EXTRACT_DIR" -Force | Out-Null
tar -xzf $FILE -C "..\$BINARYEN_EXTRACT_DIR" --strip-components=1
Pop-Location
}

function Download-WasmBindgen {
$WASM_BINDGEN_URL_BASE = "https://github.com/wasm-bindgen/wasm-bindgen/releases/download/$WASM_BINDGEN_VERSION"
$WASM_BINDGEN_DIR = "wasm-bindgen"
$ARCH = "x86_64"
$OS = "pc-windows-msvc"

$FILE = "wasm-bindgen-${WASM_BINDGEN_VERSION}-${ARCH}-${OS}.tar.gz"

Push-Location ".\.bin\tmp"
Write-Host "Downloading from $WASM_BINDGEN_URL_BASE/$FILE"
Invoke-WebRequest "$WASM_BINDGEN_URL_BASE/$FILE" -OutFile $FILE
New-Item -ItemType Directory -Path "..\$WASM_BINDGEN_DIR" -Force | Out-Null
tar -xzf $FILE -C "..\$WASM_BINDGEN_DIR" --strip-components=1
Pop-Location
}

# Ensure rustc >= 1.30.0
$MIN_RUSTC_VERSION = "1.30.0"
$RUSTC_VERSION = (& rustc --version).Split()[1]

if ([Version]$RUSTC_VERSION -lt [Version]$MIN_RUSTC_VERSION) {
Write-Error "rustc >= $MIN_RUSTC_VERSION required, found $RUSTC_VERSION"
exit 1
}

Write-Host "Using rustc version: $RUSTC_VERSION"

# Ensure wasm32 target is installed
$LIBDIR = & rustc --target wasm32-unknown-unknown --print target-libdir
if (-not (Test-Path $LIBDIR)) {
Write-Host "Target wasm32-unknown-unknown not installed. Installing..."
if (-not (Get-Command rustup -ErrorAction SilentlyContinue)) {
Write-Error "rustup not installed. Install target manually or install rustup."
exit 1
}
rustup target add wasm32-unknown-unknown
}

# Check binaryen
if (-not (Test-Path ".bin\binaryen")) {
Write-Host "Downloading binaryen..."
Download-Binaryen
}
else {
Write-Host "Found existing binaryen installation."
}

# Check wasm-bindgen
if (-not (Test-Path ".bin\wasm-bindgen")) {
Write-Host "Downloading wasm-bindgen..."
Download-WasmBindgen
}
else {
Write-Host "Found existing wasm-bindgen installation."
}

Remove-Item -Recurse -Force ".\.bin\tmp"

# Build wasm module
Write-Host "Building wasm module..."

cargo build --target wasm32-unknown-unknown --release

# Extract crate name using PowerShell JSON parsing
$metadata = & cargo metadata --format-version=1 --no-deps
$CRATE_NAME = ($metadata | ConvertFrom-Json).packages[0].name
$WASM_PATH = "target\wasm32-unknown-unknown\release\${CRATE_NAME}.wasm"

# wasm-bindgen
Write-Host "Running wasm-bindgen..."
$WASM_BINDGEN_OPTS = @("--out-dir", $OUT_DIR, "--target", $TARGET)
if (-not $GENERATE_DTS) {
$WASM_BINDGEN_OPTS += "--no-typescript"
}
& ".\.bin\wasm-bindgen\wasm-bindgen.exe" @WASM_BINDGEN_OPTS $WASM_PATH

# wasm-opt
Write-Host "Running wasm-opt..."
& ".\.bin\binaryen\bin\wasm-opt.exe" -O3 --enable-bulk-memory --enable-nontrapping-float-to-int -o "$OUT_DIR\${CRATE_NAME}_bg.wasm" "$OUT_DIR\${CRATE_NAME}_bg.wasm"

Write-Host "Build completed successfully. Output is in $OUT_DIR"

Pop-Location
181 changes: 181 additions & 0 deletions instrumentation-wasm/build.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
#!/usr/bin/env bash
set -e

# ------ CONFIGURATION ------

TARGET=nodejs
OUT_DIR=../library/agent/hooks/instrumentation/wasm
GENERATE_DTS=false

BINARYEN_VERSION=124 # wasm-opt
WASM_BINDGEN_VERSION=0.2.101

# ---------------------------

echo "Starting build process for wasm module"

# Always run from script's own directory and return at the end
SCRIPT_DIR="$(cd "$(dirname "$(realpath "$0")")" && pwd)"
pushd "$SCRIPT_DIR" > /dev/null

# Create directories

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment '# Create directories' only states what the code does; explain why these directories are needed instead.

Details

✨ AI Reasoning
​​1) The script sets up and runs a wasm build: it changes to the script directory, creates directories, defines download/build functions, downloads toolchains, builds a Cargo target, and runs wasm-bindgen/wasm-opt.
​2) The comment '# Create directories' simply restates the obvious code action (mkdir -p ...) and adds no design/intent context, which can clutter maintenance and go stale if the implementation changes.
​3) This PR introduced that comment (file added), so it is a new, redundant 'what' comment; a better comment would explain why those specific directories are needed (e.g., 'ensure output and temporary bins exist for tool downloads and build artifacts').

🔧 How do I fix it?
Write comments that explain the purpose, reasoning, or business logic behind the code using words like 'because', 'so that', or 'in order to'.

More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.

mkdir -p ./.bin/tmp
mkdir -p "$OUT_DIR"

# ------ Functions ------

download_binaryen() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function download_binaryen() implements multi-step OS/ARCH detection and archive download/extract but lacks an explanatory comment.

Details

✨ AI Reasoning
​​1) download_binaryen() downloads and extracts a Binaryen release and detects OS/ARCH.
​2) The function has multiple non-obvious branches and steps (OS/ARCH detection, URL construction, download, extraction) that would benefit from a short explanatory comment describing intent, platform assumptions, and side effects.
​3) This change introduced a sizable function without documentation, making future maintenance harder.

🔧 How do I fix it?
Add docstrings or comments explaining what the function does, its parameters, and return values.

More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.

set -e

BINARYEN_BASE_URL="https://github.com/WebAssembly/binaryen/releases/download/version_$BINARYEN_VERSION"
BINARYEN_EXTRACT_DIR="binaryen"

# Detect OS
case "$(uname -s)" in
Linux*) OS="linux" ;;
Darwin*) OS="macos" ;;
*) echo "Unsupported OS"; exit 1 ;;
esac

# Detect ARCH
case "$(uname -m)" in
x86_64) ARCH="x86_64" ;;
aarch64|arm64) ARCH="arm64" ;;
*) echo "Unsupported arch"; exit 1 ;;
esac

# Adjust ARM label for Linux
[ "$ARCH" = "arm64" ] && [ "$OS" = "linux" ] && ARCH="aarch64"

FILE="binaryen-version_${BINARYEN_VERSION}-${ARCH}-${OS}.tar.gz"

mkdir -p ./.bin/tmp

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obvious duplication: repeated download/extract/cleanup sequence appears in both download_binaryen() and download_wasm_bindgen().

Details

✨ AI Reasoning
​​1) Both download_binaryen() and download_wasm_bindgen() contain repeated sequences (mkdir -p ./.bin/tmp, cd ./.bin/tmp, download, extract, cleanup).
​2) This duplication increases maintenance burden because similar error handling or flow changes would need to be applied in multiple places.
​3) The PR introduced these two similar blocks rather than consolidating shared download/extract semantics, so the duplication was introduced/worsened by this change.

🔧 How do I fix it?
Delete extra code. Extract repeated code sequences into reusable functions or methods. Use loops or data structures to eliminate repetitive patterns.

More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.

cd ./.bin/tmp

echo "Downloading from $BINARYEN_BASE_URL/$FILE"

curl -LO --fail "$BINARYEN_BASE_URL/$FILE"

mkdir -p "../$BINARYEN_EXTRACT_DIR"
tar -xzf "$FILE" -C "../$BINARYEN_EXTRACT_DIR" --strip-components=1

cd ../..
rm -rf ./.bin/tmp
}

download_wasm_bindgen() {
set -e

WASM_BINDGEN_URL_BASE="https://github.com/wasm-bindgen/wasm-bindgen/releases/download/$WASM_BINDGEN_VERSION"
WASM_BINDGEN_DIR="wasm-bindgen"

# Detect OS and ARCH for wasm-bindgen
case "$(uname -s)" in
Linux*)
case "$(uname -m)" in
x86_64)
OS="unknown-linux-musl"
ARCH="x86_64"
;;
aarch64|arm64)
OS="unknown-linux-gnu"
ARCH="aarch64"
;;
*) echo "Unsupported arch for Linux"; exit 1 ;;
esac
;;
Darwin*)
OS="apple-darwin"
case "$(uname -m)" in
x86_64) ARCH="x86_64" ;;
aarch64|arm64) ARCH="aarch64" ;;
*) echo "Unsupported arch for Darwin"; exit 1 ;;
esac
;;
*) echo "Unsupported OS"; exit 1 ;;
esac

FILE="wasm-bindgen-${WASM_BINDGEN_VERSION}-${ARCH}-${OS}.tar.gz"

mkdir -p ./.bin/tmp
cd ./.bin/tmp

echo "Downloading from $WASM_BINDGEN_URL_BASE/$FILE"

curl -LO --fail "$WASM_BINDGEN_URL_BASE/$FILE"

mkdir -p "../$WASM_BINDGEN_DIR"
tar -xzf "$FILE" -C "../$WASM_BINDGEN_DIR" --strip-components=1

cd ../..
rm -rf ./.bin/tmp
}

# ------ Main Script ------

# Ensure rustc >= 1.30.0
MIN_RUSTC_VERSION=1.30.0
RUSTC_VERSION=$(rustc --version | cut -d' ' -f2)

# Compare versions using sort
if ! printf '%s\n%s\n' "$MIN_RUSTC_VERSION" "$RUSTC_VERSION" | sort -VC; then
echo "rustc >= $MIN_RUSTC_VERSION required, found $RUSTC_VERSION"
exit 1
fi

echo "Using rustc version: $RUSTC_VERSION"

# Ensure wasm32 target is installed
LIBDIR=$(rustc --target wasm32-unknown-unknown --print target-libdir)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling 'rustc --target ... --print target-libdir' will fail (and exit the script due to set -e) if the target isn't installed, preventing the intended installation check.

Details

✨ AI Reasoning
​​1) The script attempts to detect whether the wasm32 target is installed by running 'rustc --target wasm32-unknown-unknown --print target-libdir' and then checking if the returned path exists (lines 129-141).
2) Because 'set -e' is enabled at the top, if the wasm32 target is not installed, the rustc invocation will fail and exit non‑zero, causing the whole script to terminate before the subsequent conditional can run; the code therefore cannot reach the intended directory-existence check.
3) This contradicts the apparent intention to detect and install the target if missing, so the control flow is broken and harms the script's reliability when the target is absent.

🔧 How do I fix it?
Trace execution paths carefully. Ensure precondition checks happen before using values, validate ranges before checking impossible conditions, and don't check for states that the code has already ruled out.

More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.


if [ ! -d "$LIBDIR" ]; then
echo "Target wasm32-unknown-unknown not installed. Installing using rustup..."

# Check if rustup is installed
if ! command -v rustup &> /dev/null; then
echo "rustup is not installed. Please install the wasm32-unknown-unknown target manually or install rustup."
exit 1
fi

rustup target add wasm32-unknown-unknown
fi

# Check if binaryen is already downloaded
if [ ! -d ".bin/binaryen" ]; then
echo "Downloading binaryen..."
download_binaryen
else
echo "Found existing binaryen installation."
fi

# Check if wasm-bindgen is already downloaded
if [ ! -d ".bin/wasm-bindgen" ]; then
echo "Downloading wasm-bindgen..."
download_wasm_bindgen
else
echo "Found existing wasm-bindgen installation."
fi

# Build the wasm module
echo "Building wasm module..."
cargo build --target wasm32-unknown-unknown --release

CRATE_NAME=$(basename "$(cargo metadata --format-version=1 --no-deps | jq -r '.packages[0].name')")
WASM_PATH=target/wasm32-unknown-unknown/release/${CRATE_NAME}.wasm

# wasm-bindgen
echo "Running wasm-bindgen..."
WASM_BINDGEN_OPTS=(--out-dir "$OUT_DIR" --target "$TARGET")
if [ "$GENERATE_DTS" != true ]; then
WASM_BINDGEN_OPTS+=(--no-typescript)
fi
./.bin/wasm-bindgen/wasm-bindgen "${WASM_BINDGEN_OPTS[@]}" "$WASM_PATH"

# wasm-opt
echo "Running wasm-opt..."
./.bin/binaryen/bin/wasm-opt -O3 --enable-bulk-memory --enable-nontrapping-float-to-int -o "$OUT_DIR/${CRATE_NAME}_bg.wasm" "$OUT_DIR/${CRATE_NAME}_bg.wasm"

echo "Build completed successfully. Output is in $OUT_DIR"

# Switch back to the original directory
popd > /dev/null
1 change: 1 addition & 0 deletions library/.oxlintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"no-prototype-builtins": "error",
"require-await": "error"
},
"ignorePatterns": ["**/wasm/node_code_instrumentation.js"],
"overrides": [
{
"files": ["**/*.ts"],
Expand Down
20 changes: 8 additions & 12 deletions scripts/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,18 +149,14 @@ async function modifyDtsFilesAfterBuild() {

async function buildInstrumentationWasm() {
// Build Instrumentation WASM
await execAsyncWithPipe(
`wasm-pack build --release --target nodejs --out-dir ${instrumentationWasmOutDir}`,
{
cwd: instrumentationWasmDir,
}
);

// Delete .d.ts files generated by wasm-pack
await rm(join(instrumentationWasmOutDir, "node_code_instrumentation.d.ts"));
await rm(
join(instrumentationWasmOutDir, "node_code_instrumentation_bg.wasm.d.ts")
);
const isWindows = process.platform === "win32";
const buildScript = isWindows ? "./build.ps1" : "./build.sh";
const command = isWindows
? `pwsh -File ${buildScript}`
: `bash ${buildScript}`;
await execAsyncWithPipe(command, {
cwd: instrumentationWasmDir,
});
}

(async () => {
Expand Down
Loading