Build the build#1
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a TypeScript AWS Lambda handler that forwards request bodies to a Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
package.json (1)
5-5: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse Node resolution that matches the package module type.
"type": "module"makes Node treathandler.jsas ESM, whilemodule: "commonjs"emits CommonJS with arequirecall under"bundler"resolution. Usemodule: "NodeNext"withmoduleResolution: "NodeNext", or keep CommonJS by removing"type": "module"and using Node/CommonJS settings consistently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 5, Align package.json and tsconfig.json module settings: either retain "type": "module" and change compilerOptions.module and compilerOptions.moduleResolution to NodeNext, or remove "type": "module" and configure consistent CommonJS/Node resolution. Update all three referenced sites accordingly so emitted handler.js syntax matches Node’s runtime interpretation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-release.yml:
- Around line 45-57: Serialize the workflow’s shared “latest” release update to
prevent concurrent runs from overwriting it with stale artifacts. Add a
workflow-level concurrency group with cancellation or otherwise gate the “Update
latest release” step so only the newest main-branch commit performs the update,
while preserving independent build behavior.
- Line 32: Update the release step using softprops/action-gh-release from `@v1` to
the Node-20-compatible `@v3` version, and apply the same upgrade to any shared
release workflow usage.
In `@build_layer.sh`:
- Around line 108-114: Update the layer size validation in the “Warn if layer is
too large” block to sum the uncompressed sizes of all entries in LAYER_ZIP
rather than using du on the compressed archive. Convert the ZIP entry total to
megabytes, compare it against the 250 MB limit, and retain the existing warning,
current-size output, and failure behavior.
- Around line 72-93: Update the layer packaging flow in build_layer.sh so the
ZIP contains Node dependencies under nodejs/node_modules and the application
distribution under the corresponding nodejs path expected by Lambda. Also expose
the verified humio-mcp CLI through a top-level bin directory on the layer PATH,
or configure callers to spawn its explicit /opt path instead of relying on
node_modules/.bin; keep the existing packaging error handling intact.
- Around line 46-47: The upstream Humio MCP build must not execute mutable code
with write credentials: in build_layer.sh lines 46-47, replace the main ref with
the reviewed immutable commit SHA; in build_layer.sh lines 61-67, use the
upstream lockfile with a lockfile-based, integrity-checked install; and in
.github/workflows/build-release.yml lines 18-21, set read-only build permissions
with persist-credentials=false, isolating contents:write to a release job that
consumes the build artifact.
- Around line 34-41: Update the release packaging flow around the handler.js
copy and layer ZIP creation so the published release includes a deployable
handler artifact. Either produce and publish a separate handler artifact
consumed by Terraform, or include handler.js in the documented layer release
contract; ensure the upload path and consumer expectations consistently
reference the resulting artifact.
In `@handler.ts`:
- Around line 52-62: Bound buffering in the subprocess handling flow around the
stdout data listener: track accumulated output bytes, stop appending once the
configured response limit is reached, kill mcpProcess, and resolve with an
explicit oversized-output error. Apply equivalent protection to stderr if it is
also unbounded, while preserving normal collection and completion behavior below
the limit.
- Around line 47-49: Update the subprocess timeout and close-handling flow
around mcpProcess.kill and the close handler: track when the timeout fires,
clear the timer when the process closes, and return HTTP 504 after a timeout
instead of processing partial output. Also treat any signal termination
(non-null signal) as a process failure rather than returning a successful 200
response.
In `@README.md`:
- Around line 7-13: Update the README workflow description to state that commits
to main trigger the GitHub Action only when the configured path filters match,
rather than implying every commit publishes a release. Preserve the existing
four-step build and publishing summary.
---
Nitpick comments:
In `@package.json`:
- Line 5: Align package.json and tsconfig.json module settings: either retain
"type": "module" and change compilerOptions.module and
compilerOptions.moduleResolution to NodeNext, or remove "type": "module" and
configure consistent CommonJS/Node resolution. Update all three referenced sites
accordingly so emitted handler.js syntax matches Node’s runtime interpretation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2efa753f-d1ca-45cb-a5ef-143c8d6d61c9
📒 Files selected for processing (6)
.github/workflows/build-release.ymlREADME.mdbuild_layer.shhandler.tspackage.jsontsconfig.json
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
| run: bash build_layer.sh | ||
|
|
||
| - name: Create release | ||
| uses: softprops/action-gh-release@v1 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow excerpt =="
sed -n '1,90p' .github/workflows/build-release.yml 2>/dev/null || true
echo "== workflows in repo =="
git ls-files .github/workflowsRepository: netlify/humio-mcp-build-lambda
Length of output: 2068
🌐 Web query:
softprops/action-gh-release v1 Node 20 runner support actionlint cannot run
💡 Result:
The softprops/action-gh-release action has officially moved to Node 24 support with the release of version 3.0.0 [1][2]. Node.js 20 actions are deprecated on GitHub Actions [2], and starting June 2, 2026, actions are being forced to run on Node.js 24 by default [2]. If you are encountering issues or receiving deprecation warnings, you should upgrade your workflow to use the latest version of the action [1][2]. 1. Update your workflow: Change your usage from softprops/action-gh-release@v1 or @v2 to softprops/action-gh-release@v3 [3][4][5]. 2. Actionlint compatibility: If you receive errors from actionlint regarding the action, ensure you are using a recent version of actionlint. Actionlint typically validates workflow syntax against current GitHub Actions runner environments and specifications [6][7]. If the error persists, it may be due to cached metadata or a configuration issue in your repository, rather than a failure of the action itself to run [8][9]. Version v2.6.2 remains available if you absolutely require a final Node 20-compatible version, but migration to v3 is strongly recommended due to the deprecation of Node 20 [5].
Citations:
- 1: softprops/action-gh-release@b430933
- 2: release: cut v3.0.0 for Node 24 upgrade softprops/action-gh-release#670
- 3: https://github.com/softprops/action-gh-release?tab=readme-ov-file
- 4: https://github.com/marketplace/actions/gh-release
- 5: https://github.com/fourcels/action-gh-release/blob/4dd30955cca228da305ffee6fbc3fc1f95d447c2/CHANGELOG.md
- 6: https://github.com/rhysd/actionlint/blob/v1.7.12/docs/checks.md
- 7: https://github.com/rhysd/actionlint/blob/main/docs/checks.md
- 8: https://github.com/rhysd/actionlint/blob/main/README.md
- 9: https://github.com/rhysd/actionlint/tree/refs/heads/main
Stop the workflow from using softprops/action-gh-release@v1.
softprops/action-gh-release@v1 uses a Node.js 20 runtime that is being phased out from GitHub Actions runners. Upgrade this step, and any shared release usage, to softprops/action-gh-release@v3 (or the documented Node-20-compatible v2.6.2 if Node 20 support is required).
🧰 Tools
🪛 actionlint (1.7.12)
[error] 32-32: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 zizmor (1.26.1)
[info] 32-32: action functionality is already included by the runner (superfluous-actions): use gh release in a script step
(superfluous-actions)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build-release.yml at line 32, Update the release step
using softprops/action-gh-release from `@v1` to the Node-20-compatible `@v3`
version, and apply the same upgrade to any shared release workflow usage.
Source: Linters/SAST tools
| - name: Update latest release | ||
| uses: softprops/action-gh-release@v1 | ||
| with: | ||
| files: .terraform/humio_mcp_layer.zip | ||
| tag_name: latest | ||
| name: humio-mcp-layer (latest) | ||
| body: | | ||
| Latest humio-mcp Lambda layer build | ||
|
|
||
| **Commit**: ${{ github.sha }} | ||
| **Built**: ${{ github.server_url }}/${{ github.repository }}/commit/${{ github.sha }} | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} No newline at end of file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Serialize updates to the shared latest release.
Two pushes can build concurrently; an older run that finishes last overwrites the latest asset with an older commit. Add a workflow-level concurrency group or otherwise ensure only the newest main commit performs this update.
🧰 Tools
🪛 actionlint (1.7.12)
[error] 46-46: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 zizmor (1.26.1)
[info] 46-46: action functionality is already included by the runner (superfluous-actions): use gh release in a script step
(superfluous-actions)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build-release.yml around lines 45 - 57, Serialize the
workflow’s shared “latest” release update to prevent concurrent runs from
overwriting it with stale artifacts. Add a workflow-level concurrency group with
cancellation or otherwise gate the “Update latest release” step so only the
newest main-branch commit performs the update, while preserving independent
build behavior.
| HUMIO_MCP_REPO="https://github.com/islam3zzat/humio-mcp.git" | ||
| HUMIO_MCP_REF="main" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Do not run mutable third-party code with persisted write credentials.
The release job checks out with contents: write, clones an unpinned external main, then executes its npm lifecycle/build scripts. A compromised upstream dependency can use the persisted checkout credential to modify this repository. Pin the upstream commit, set persist-credentials: false, and split untrusted builds into a read-only job; grant contents: write only to a release job consuming its artifact. (github.com)
build_layer.sh#L46-L47: replacemainwith a reviewed immutable commit SHA.build_layer.sh#L61-L67: use lockfile-based, integrity-checked installation where the upstream project provides a lockfile..github/workflows/build-release.yml#L18-L21: use read-only build permissions andpersist-credentials: false; isolate release permissions.
📍 Affects 2 files
build_layer.sh#L46-L47(this comment)build_layer.sh#L61-L67.github/workflows/build-release.yml#L18-L21
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build_layer.sh` around lines 46 - 47, The upstream Humio MCP build must not
execute mutable code with write credentials: in build_layer.sh lines 46-47,
replace the main ref with the reviewed immutable commit SHA; in build_layer.sh
lines 61-67, use the upstream lockfile with a lockfile-based, integrity-checked
install; and in .github/workflows/build-release.yml lines 18-21, set read-only
build permissions with persist-credentials=false, isolating contents:write to a
release job that consumes the build artifact.
Source: Linters/SAST tools
| # Copy node_modules and dist to layer build directory | ||
| echo "Packaging layer dependencies..." | ||
| mkdir -p "$BUILD_DIR/node_modules" | ||
| cp -r "$HUMIO_MCP_SOURCE/node_modules" "$BUILD_DIR/" || { | ||
| echo "Error: Failed to copy node_modules" | ||
| exit 1 | ||
| } | ||
|
|
||
| mkdir -p "$BUILD_DIR/dist" | ||
| cp -r "$HUMIO_MCP_SOURCE/dist" "$BUILD_DIR/" || { | ||
| echo "Error: Failed to copy dist" | ||
| exit 1 | ||
| } | ||
|
|
||
| # Create layer ZIP | ||
| echo "Creating Lambda layer ZIP..." | ||
| cd "$BUILD_DIR" | ||
| if [ -f "$LAYER_ZIP" ]; then | ||
| rm "$LAYER_ZIP" | ||
| fi | ||
|
|
||
| zip -r -q "$LAYER_ZIP" node_modules dist || { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Package dependencies and the CLI on Lambda search paths.
This ZIP extracts to /opt/node_modules and /opt/dist, but Node layers require nodejs/node_modules; spawn('humio-mcp') also searches PATH, which includes /opt/bin, not node_modules/.bin. Every invocation can fail with ENOENT. Package modules under nodejs/ and expose the verified CLI under bin/ (or spawn its explicit /opt/... path). (docs.aws.amazon.com)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build_layer.sh` around lines 72 - 93, Update the layer packaging flow in
build_layer.sh so the ZIP contains Node dependencies under nodejs/node_modules
and the application distribution under the corresponding nodejs path expected by
Lambda. Also expose the verified humio-mcp CLI through a top-level bin directory
on the layer PATH, or configure callers to spawn its explicit /opt path instead
of relying on node_modules/.bin; keep the existing packaging error handling
intact.
| # Warn if layer is too large | ||
| LAYER_SIZE_MB=$(du -m "$LAYER_ZIP" | cut -f1) | ||
| if [ "$LAYER_SIZE_MB" -gt 250 ]; then | ||
| echo "Warning: Layer exceeds 250MB. Lambda layers have a 250MB uncompressed limit." | ||
| echo "Current size: ${LAYER_SIZE_MB}MB" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Check the uncompressed package size.
du -m "$LAYER_ZIP" measures the compressed archive, not extracted contents. A highly compressible layer can pass this check and then be rejected by Lambda’s 250 MB unzipped deployment limit. Calculate the ZIP’s uncompressed entry total instead. (docs.aws.amazon.com)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build_layer.sh` around lines 108 - 114, Update the layer size validation in
the “Warn if layer is too large” block to sum the uncompressed sizes of all
entries in LAYER_ZIP rather than using du on the compressed archive. Convert the
ZIP entry total to megabytes, compare it against the 250 MB limit, and retain
the existing warning, current-size output, and failure behavior.
| let output = ''; | ||
| let errorOutput = ''; | ||
|
|
||
| return new Promise((resolve) => { | ||
| mcpProcess.stdout?.on('data', (data) => { | ||
| output += data.toString(); | ||
| }); | ||
|
|
||
| mcpProcess.stderr?.on('data', (data) => { | ||
| errorOutput += data.toString(); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound buffered subprocess output.
output += data.toString() has no limit. A large query response can exhaust Lambda memory or exceed the 6 MB synchronous response limit; cap bytes while streaming, kill the child, and return an explicit error. (docs.aws.amazon.com)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@handler.ts` around lines 52 - 62, Bound buffering in the subprocess handling
flow around the stdout data listener: track accumulated output bytes, stop
appending once the configured response limit is reached, kill mcpProcess, and
resolve with an explicit oversized-output error. Apply equivalent protection to
stderr if it is also unbounded, while preserving normal collection and
completion behavior below the limit.
| This repository contains the build configuration for the humio-mcp Lambda layer. On each commit to `main`, GitHub Actions: | ||
|
|
||
| 1. Compiles the TypeScript handler | ||
| 2. Clones humio-mcp from GitHub | ||
| 3. Packages everything as a Lambda layer ZIP | ||
| 4. Publishes to GitHub releases | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the path-filtered trigger.
The workflow does not run on every commit to main; it runs only when the listed paths change. Reword this to avoid implying that unrelated commits publish a new release.
Also applies to: 25-27
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 7 - 13, Update the README workflow description to
state that commits to main trigger the GitHub Action only when the configured
path filters match, rather than implying every commit publishes a release.
Preserve the existing four-step build and publishing summary.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build_layer.sh`:
- Around line 106-113: Update the runtime config copy logic in build_layer.sh to
require and use the deployment-specific HUMIO_QUERY_CONFIG file instead of
humio-query-config.example.json. Preserve the existing destination
"$BUILD_DIR/humio-query-config.json" and failure handling, while ensuring the
production layer cannot fall back to upstream example queries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 72f4f7be-ba34-4cd7-ad57-c464859d94c5
📒 Files selected for processing (2)
build_layer.shhandler.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- handler.ts
| # Copy the example query config as the runtime config. JsonConfigProvider | ||
| # resolves "humio-query-config.json" relative to its own module location, | ||
| # three directories up from dist/humio/config -- i.e. the layer root. | ||
| # Without this file present, the server fails to start. | ||
| cp "$HUMIO_MCP_SOURCE/humio-query-config.example.json" "$BUILD_DIR/humio-query-config.json" || { | ||
| echo "Error: Failed to copy humio-query-config.json" | ||
| exit 1 | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files relevant =="
git ls-files | rg '(^|/)(build_layer\.sh|humio-query-config(\.example)?\.json|package\.json|README\.md)$' || true
echo
echo "== build_layer.sh outline/sections =="
wc -l build_layer.sh
sed -n '1,180p' build_layer.sh
echo
echo "== example config =="
if [ -f humio-query-config.example.json ]; then
cat -n humio-query-config.example.json
fi
echo
echo "== config references in repo =="
rg -n "humio-query-config|HUMIO_QUERY_CONFIG|banana|team|JsonConfigProvider|dist/humio/config" . --glob '!node_modules' --glob '!dist' || trueRepository: netlify/humio-mcp-build-lambda
Length of output: 5140
🌐 Web query:
islam3zzat humio-mcp humio-query-config.example.json banana team
💡 Result:
The references to humio-query-config.example.json and the "banana team" are associated with a specific implementation of a Humio Model Context Protocol (MCP) server created by user islam3zzat [1]. This MCP server is designed to allow LLMs or other clients to execute pre-defined queries against Humio (Falcon LogScale) [1]. The humio-query-config.example.json file serves as a template that users populate to define the queries available to the server [1]. The "banana team" example included in this template demonstrates how to structure query configurations, specifically filtering for logs associated with a team label and grouping the results [1]. Key details regarding the humio-query-config.json structure include [1]: - name: A unique identifier for the query. - description: A human-readable explanation of the query's purpose (e.g., "Finds critical errors for the banana team"). - query: The actual Humio query string, which may include placeholders for variables (e.g., {{variableName}}). - fields: An array of fields to extract from the query results. - outputTemplate: A string template for formatting the results, often using placeholders for the extracted fields. - joinString: A separator used when multiple results are combined. - variables: A list of definitions for any parameters required by the query, such as type and requirement status. Users typically initialize their configuration by copying the example file: cp humio-query-config.example.json humio-query-config.json [1]. Other Humio MCP implementations exist (e.g., by weihong-xu-hpe), but they often use different configuration formats, such as TOML-based files [2][3][4].
Citations:
- 1: https://github.com/islam3zzat/humio-mcp
- 2: https://github.com/weihong-xu-hpe/humio-mcp
- 3: https://github.com/weihong-xu-hpe/humio-mcp/blob/main/README.md
- 4: https://lobehub.com/mcp/weihong-xu-hpe-humio-mcp
🌐 Web query:
github.com islam3zzat humio-mcp humio-query-config.example.json
💡 Result:
The file humio-query-config.example.json is a configuration template used by the humio-mcp server repository maintained by islam3zzat [1]. It serves as a schema and example for defining pre-configured queries that the Humio MCP server can expose to MCP-compatible clients [1]. Key details regarding this file and the humio-mcp project: Purpose: The humio-query-config.json (derived from the example) defines a list of available, predefined queries [1]. This allows users to store complex or frequently used Humio/LogScale queries in a structured format rather than writing raw queries repeatedly [1]. Configuration Structure: Each query entry in the JSON array requires the following fields [1]: - name: A unique identifier for the query [1]. - description: A human-readable explanation of the query's purpose [1]. - queryString: The actual Humio (Falcon LogScale) query string, which supports variables (e.g., {{variable}}) for dynamic interpolation [1]. - fields: A list of result fields to extract [1]. - outputTemplate: A string template used for formatting query results, supporting variable interpolation [1]. - joinString: A string used to join multiple formatted results [1]. Setup: To use this configuration, users typically copy the example file to the active configuration file [1]: cp humio-query-config.example.json humio-query-config.json Note: Several other "Humio MCP" projects exist (such as those by weihong-xu-hpe and bhayanak), but they typically utilize TOML-based configurations for cluster and authentication settings rather than the specific JSON-based query configuration file described in the islam3zzat repository [2][3][4][5].
Citations:
- 1: https://github.com/islam3zzat/humio-mcp
- 2: https://github.com/weihong-xu-hpe/humio-mcp/blob/main/README.md
- 3: https://github.com/weihong-xu-hpe/humio-mcp
- 4: https://github.com/bhayanak/logscale-mcp-server
- 5: https://lobehub.com/mcp/weihong-xu-hpe-humio-mcp
Use a deployment-specific runtime config instead of the upstream example.
humio-query-config.example.json is a template, and the copied active config can include environment-specific queries such as the banana team filters. Requiring HUMIO_QUERY_CONFIG for the production config prevents the Lambda layer from shipping data-scoped examples.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build_layer.sh` around lines 106 - 113, Update the runtime config copy logic
in build_layer.sh to require and use the deployment-specific HUMIO_QUERY_CONFIG
file instead of humio-query-config.example.json. Preserve the existing
destination "$BUILD_DIR/humio-query-config.json" and failure handling, while
ensuring the production layer cannot fall back to upstream example queries.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
handler.ts (1)
89-114: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUnbounded stdout/stderr buffering still unaddressed.
output += data.toString()(Line 90) anderrorOutput += data.toString()(Line 113) have no size cap. A large query response accumulates without bound until a newline appears or the process closes, risking Lambda memory exhaustion or exceeding the 6 MB synchronous response limit. This was flagged in a prior review and remains unresolved in this version.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@handler.ts` around lines 89 - 114, Cap accumulated stdout and stderr data in the mcpProcess stdout and stderr handlers before appending to output and errorOutput. Enforce bounded buffering that prevents Lambda memory growth and keeps the eventual response within the synchronous size limit, while preserving completion on the first complete newline-delimited stdout message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@handler.ts`:
- Around line 125-134: Update the mcpProcess close handler to treat a null exit
code as a failure when determining the response status, so signal-terminated
processes return 502 rather than 200. Preserve the existing success status for
code 0 and the current finish() short-circuit behavior for deliberate
termination.
- Line 35: Remove raw payload logging around the request handling flow,
including the console.log that prints body and the stdout/stderr logging at the
MCP response handling lines. If operational logging is required, replace it with
non-sensitive metadata such as request status or identifiers, without
serializing query parameters, response content, or full JSON-RPC payloads.
- Around line 136-140: Attach an error listener to mcpProcess.stdin before
writing the request in the stdin write path, and route the emitted stream error
through the existing error-handling flow so it is captured rather than becoming
unhandled. Keep the current newline normalization and deliberate omission of
stdin.end() unchanged.
---
Duplicate comments:
In `@handler.ts`:
- Around line 89-114: Cap accumulated stdout and stderr data in the mcpProcess
stdout and stderr handlers before appending to output and errorOutput. Enforce
bounded buffering that prevents Lambda memory growth and keeps the eventual
response within the synchronous size limit, while preserving completion on the
first complete newline-delimited stdout message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d1da51d7-82c1-4c17-b8e3-e652d1533a0a
📒 Files selected for processing (1)
handler.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
| }; | ||
| } | ||
|
|
||
| console.log('humio-mcp request:', body); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Logging full request/response bodies may leak sensitive query data.
console.log('humio-mcp request:', body) and the stdout/stderr logs at Lines 100-103 write the raw JSON-RPC payload and the raw MCP response line verbatim to CloudWatch. Since this proxies Humio log-query traffic, both the request (query parameters) and the response (query results) can contain sensitive/PII data that then persists in logs outside any Humio access controls.
Suggested mitigation
- console.log('humio-mcp request:', body);
+ console.log('humio-mcp request received, length:', body.length);
...
- console.log('humio-mcp stdout:', line);
- if (errorOutput) {
- console.log('humio-mcp stderr:', errorOutput);
- }
+ console.log('humio-mcp response length:', line.length);Also applies to: 100-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@handler.ts` at line 35, Remove raw payload logging around the request
handling flow, including the console.log that prints body and the stdout/stderr
logging at the MCP response handling lines. If operational logging is required,
replace it with non-sensitive metadata such as request status or identifiers,
without serializing query parameters, response content, or full JSON-RPC
payloads.
| mcpProcess.on('close', (code) => { | ||
| if (code !== 0 && code !== null) { | ||
| console.error(`humio-mcp exited with code ${code}: ${errorOutput}`); | ||
| } | ||
| finish({ | ||
| statusCode: code === 0 || code === null ? 200 : 502, | ||
| headers: { 'content-type': 'application/json' }, | ||
| body: output || JSON.stringify({ error: 'MCP server produced no output', stderr: errorOutput }), | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Signal-terminated subprocess still returns 200, and can pair a "no output" error body with a success status.
code === 0 || code === null ? 200 : 502 treats any null exit code as success, including an unexpected SIGKILL/crash (as opposed to finish()'s own deliberate kill, which is already short-circuited by settled). If the process is killed by signal before producing output, this resolves 200 with body: JSON.stringify({ error: 'MCP server produced no output', ... }) — a success status carrying an error-shaped body. A prior review flagged this exact "treat signal termination as a process failure" gap; the timeout-specific case was fixed via finish(), but the general code === null (non-timeout) path was not.
Suggested fix
- mcpProcess.on('close', (code) => {
- if (code !== 0 && code !== null) {
+ mcpProcess.on('close', (code, signal) => {
+ if (code !== 0) {
console.error(`humio-mcp exited with code ${code}: ${errorOutput}`);
}
finish({
- statusCode: code === 0 || code === null ? 200 : 502,
+ statusCode: code === 0 ? 200 : 502,
headers: { 'content-type': 'application/json' },
body: output || JSON.stringify({ error: 'MCP server produced no output', stderr: errorOutput }),
});
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@handler.ts` around lines 125 - 134, Update the mcpProcess close handler to
treat a null exit code as a failure when determining the response status, so
signal-terminated processes return 502 rather than 200. Preserve the existing
success status for code 0 and the current finish() short-circuit behavior for
deliberate termination.
| // Send request to humio-mcp stdin. Deliberately not calling | ||
| // stdin.end() -- see finish() above. | ||
| if (mcpProcess.stdin) { | ||
| mcpProcess.stdin.write(body.endsWith('\n') ? body : body + '\n'); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== handler outline =="
ast-grep outline handler.ts || true
echo "== relevant handler section =="
cat -n handler.ts | sed -n '80,165p'
echo "== mcp process spawn/stdin usages =="
rg -n "spawn|mcpProcess|stdin|EPIPE|write\\(" handler.tsRepository: netlify/humio-mcp-build-lambda
Length of output: 4424
🏁 Script executed (no clone):
#!/bin/bash
set -euo pipefail
echo "== EventEmmiter handling of unhandled stream errors in current Node =="
node - <<'JS'
console.log(process.version);
const stdin = process.stdin;
console.log('hasErrorListenerBefore:', stdin.listenerCount('error'));
setImmediate(() => {
stdin.emit('error', new Error('handled later'));
console.log('hasErrorListenerAfter:', stdin.listenerCount('error'));
});
setTimeout(() => {
console.log('still running');
process.exit(0);
}, 50);
setTimeout(() => {
console.log('timeout exited');
process.exit(0);
}, 500);
JS
echo "== ReadableStream stdin write behavior simulation =="
node - <<'JS'
const stream = require('stream');
const { ChildProcess } = require('child_process');
const p = new ChildProcess({ stdio: ['pipe', 'pipe', 'pipe'] });
console.log('stdinWritable:', p.stdin.writable);
p.stdin.once('close', () => console.log('stdin closed'));
let wrote = false, emitted = false;
try {
const ok = p.stdin.write('x');
wrote = ok;
} catch (err) {
console.log('throw:' + err.constructor.name + ':' + err.message, true);
}
console.log('wrote:' + wrote + ' hasErrorListener:' + p.stdin.listenerCount('error'));
setTimeout(() => {
console.log('emittedError:' + emitted + ' hasErrorListenerAfter:' + p.stdin.listenerCount('error'));
if (p.stdin.listenerCount('error') === 0) process.exit(2); else process.exit(0);
}, 20);
JS
Length of output: 594
🌐 Web query:
Node.js writable stream write after child exits EPIPE unhandled error listener documentation
💡 Result:
In Node.js, an EPIPE error (Broken Pipe) occurs when you attempt to write to a writable stream (such as a child process's stdin or a network socket) after the reading end has closed [1][2]. Because Node.js handles this as an error event, an unhandled EPIPE event will cause the entire process to crash with an uncaught exception [1][2]. Key points regarding EPIPE and error handling: Mechanism of Crash: If no 'error' listener is attached to a writable stream, any error emitted by that stream—including EPIPE—is treated as an uncaught exception [1][2]. This design forces developers to explicitly manage stream errors [1]. Handling EPIPE: To prevent the process from crashing, you must attach an 'error' event listener to the writable stream [3][1]. It is common practice to check for the EPIPE error code and handle it gracefully [1]: stream.on('error', (err) => { if (err.code === 'EPIPE') { // Gracefully ignore or handle the broken pipe console.error('Remote end closed connection; ignoring EPIPE.'); } else { // Handle other types of errors throw err; } }); Contexts where EPIPE is common: - Child Processes: If you write to a child process's stdin after the process has exited or closed its stdin, the write will fail with EPIPE [4][5][6]. - Shell Pipes: When running a Node.js script in a shell pipeline (e.g., node script.js | head), the receiving process (head) may close its end early, causing the Node.js process to receive an EPIPE when it attempts to write to process.stdout [1]. Best Practices: - Always attach an 'error' listener to writable streams you interact with, especially those connected to child processes or external network sockets [1]. - For piping (a.pipe(b)), note that errors are not automatically propagated from the source to the destination [7][8]. You should attach listeners to all streams in a pipeline or use stream.pipeline to manage error propagation and cleanup automatically [7]. - Before writing, you can check the stream's state (e.g., stream.writableEnded, stream.destroyed) to avoid writing to a closed stream, although this does not eliminate the need for an 'error' listener as a safety measure [1].
Citations:
- 1: https://nodejs-errors.dev/EPIPE/
- 2: https://nodejs.org/api/errors.html
- 3: https://techqa.club/v/q/node-js-an-uncatchable-error-is-thrown-when-the-child-process-is-abruptly-closed-during-a-large-write-operation-to-it-s-stdin-stream-67296866
- 4: child_process: exec EPIPE nodejs/node#29206
- 5:
EPIPEwhen writing to newly created child process's stdin nodejs/node#40085 - 6: writing to a child subprocess nodejs/node#21365
- 7: https://stackoverflow.com/questions/21771220/error-handling-with-node-js-streams
- 8: https://www.bennadel.com/blog/2679-how-error-events-affect-piped-streams-in-node-js.htm
Attach an error handler to mcpProcess.stdin.
The handler has error handling for the child process itself, but not for stdin. If the subprocess exits or closes stdin before the write completes, the write emits an unhandled stream error and crashes the Node process instead of being captured in the existing try/catch.
Suggested fix
if (mcpProcess.stdin) {
+ mcpProcess.stdin.on('error', (err) => {
+ console.error('humio-mcp stdin error:', err);
+ });
mcpProcess.stdin.write(body.endsWith('\n') ? body : body + '\n');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Send request to humio-mcp stdin. Deliberately not calling | |
| // stdin.end() -- see finish() above. | |
| if (mcpProcess.stdin) { | |
| mcpProcess.stdin.write(body.endsWith('\n') ? body : body + '\n'); | |
| } | |
| // Send request to humio-mcp stdin. Deliberately not calling | |
| // stdin.end() -- see finish() above. | |
| if (mcpProcess.stdin) { | |
| mcpProcess.stdin.on('error', (err) => { | |
| console.error('humio-mcp stdin error:', err); | |
| }); | |
| mcpProcess.stdin.write(body.endsWith('\n') ? body : body + '\n'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@handler.ts` around lines 136 - 140, Attach an error listener to
mcpProcess.stdin before writing the request in the stdin write path, and route
the emitted stream error through the existing error-handling flow so it is
captured rather than becoming unhandled. Keep the current newline normalization
and deliberate omission of stdin.end() unchanged.
No description provided.