docs(agent): document structured event schemas and add validation tests - #479
docs(agent): document structured event schemas and add validation tests#479samueldoo wants to merge 3 commits into
Conversation
- Add comprehensive OpenAPI 3.1 spec covering all routes in registry.js, agents.js, services.js, and demo.js - Serve OpenAPI spec at /openapi.json endpoint - Add contract tests to validate API responses against OpenAPI spec - Install @apidevtools/swagger-parser for spec validation - Document all request/response schemas with proper types and validation
- Add url-validator.js with comprehensive URL validation - Block non-HTTPS endpoints - Block private, loopback, and link-local IP addresses (IPv4 and IPv6) - Resolve DNS and block endpoints that resolve to private IPs - Add configurable ALLOWED_ENDPOINTS allowlist for local development - Validate redirects to prevent SSRF via redirect chains - Integrate validation into agent.js HTTP client - Add comprehensive test suite covering SSRF attack scenarios - Tests cover AWS metadata service, localhost, and DNS rebinding attacks
|
@samueldoo Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughThe PR documents and tests agent event schemas, adds SSRF protections to agent HTTP requests, and introduces a served OpenAPI 3.1 specification with contract tests for backend routes and schemas. ChangesAgent event contracts
SSRF-protected agent requests
OpenAPI contract
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Agent as agent.js
participant SafeFetch as safeFetch
participant Validator as URL validator
participant Endpoint as Remote endpoint
Agent->>SafeFetch: Send probe or payment request
SafeFetch->>Validator: Validate target URL
Validator-->>SafeFetch: Return validation result
SafeFetch->>Endpoint: Fetch with manual redirects
Endpoint-->>SafeFetch: Return response or redirect
SafeFetch->>Validator: Validate redirect target
SafeFetch-->>Agent: Return validated response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (3)
backend/test/openapi.test.js (1)
124-177: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftConsider validating bodies against the spec schemas instead of property presence.
These assertions only check key presence, so type/enum/required drift in
openapi.jsongoes undetected. Compiling the resolved component schemas with Ajv (2020-12 for 3.1) againstresponse.bodywould make the contract tests actually enforce the contract.🤖 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 `@backend/test/openapi.test.js` around lines 124 - 177, Replace the property-presence checks in the Response Schema Validation tests with actual validation of each response.body against its corresponding resolved OpenAPI component schema. Configure Ajv for OpenAPI 3.1’s JSON Schema 2020-12 dialect, compile HealthResponse, ServicesListResponse, AgentsListResponse, RegistryStats, and Error schemas, and assert validation succeeds while preserving the existing status-code and conditional agents behavior.backend/src/index.js (1)
93-106: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueLoad the OpenAPI spec once at startup once the runtime version can be ensured.
backend/package.jsondeclaresnode >=22.0.0, so thewith: { type: "json" }import syntax is supported; if the deployed runtime can be locked to>=22.12.0, loading../openapi.jsononce frombackend/openapi.jsonand serving the cached spec avoids runtime exception handling and async per-request work.🤖 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 `@backend/src/index.js` around lines 93 - 106, Update the startup initialization and the /openapi.json handler to load ../openapi.json once after confirming the runtime meets the required Node version, cache its default export, and serve the cached spec synchronously. Remove the per-request dynamic import and its associated runtime exception handling while preserving the existing JSON response behavior.agent/agent.js (1)
195-209: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant SSRF validation —
safeFetchalready re-validates the same URL.
createSafeFetch(Line 199 inurl-validator.js) callsvalidateEndpointUrlinternally before every request. The manualvalidateEndpointUrlcall here duplicates that check (and its DNS lookup) beforesafeFetchis even invoked at Line 209, adding latency and duplicated error-handling logic without extra protection (the inner check runs again regardless).♻️ Suggested simplification
const safeFetch = createSafeFetch(fetch); httpClient.fetch = async (url, init = {}) => { - // Validate endpoint URL before making any request - const validation = await validateEndpointUrl(url); - if (!validation.valid) { - logger.warn({ url, reason: validation.reason }, 'Endpoint URL blocked by SSRF protection'); - const err = new Error(validation.reason); - err.code = 'SSRF_BLOCKED'; - throw err; - } - - const probe = await safeFetch(url, init); + const probe = await safeFetch(url, init).catch((err) => { + logger.warn({ url, reason: err.message }, 'Endpoint URL blocked by SSRF protection'); + throw err; + });🤖 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 `@agent/agent.js` around lines 195 - 209, Remove the manual validateEndpointUrl call and its duplicate SSRF_BLOCKED warning/error handling from the httpClient.fetch wrapper. Rely on safeFetch, created via createSafeFetch, to perform endpoint validation before each request, while preserving the existing request flow and error propagation.
🤖 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 `@agent/agent.test.js`:
- Around line 601-626: Update validateEvent to enforce the schema’s allowed
fields by combining schema.required and schema.optional, rejecting payload
fields outside that set. Preserve required-field and type validation, and
explicitly include the documented logger metadata fields in the permitted set
where applicable.
- Around line 590-593: Update the AGENT_COMPLETE schema in agent/agent.test.js
so finalScore and scoreDelta accept number or null, and add a disabled-scoring
test exercising the emitted null values. Update the corresponding AGENT_COMPLETE
example and field documentation in agent/README.md to describe these fields as
number | null.
In `@agent/README.md`:
- Around line 323-340: Update the README JSON log example to consume raw log
lines from an actual readable stream or Pino destination instead of accessing
logger.stream from the pino-pretty transport. Ensure the input remains
newline-delimited JSON before passing each line to JSON.parse, while keeping the
existing log-processing flow intact.
In `@agent/url-validator.js`:
- Line 137: Replace substring-based allowlist checks in the hostname validation
logic and validateRedirect with exact-domain or subdomain-boundary matching:
accept the allowlisted hostname itself or a hostname ending with "." plus the
allowlisted domain, while rejecting attacker-controlled suffix and prefix
variants.
- Around line 150-162: Update validateEndpointUrl and createSafeFetch so DNS
resolution is performed once and the validated address is pinned to the actual
HTTP connection. Reject any private or unsafe resolved IP, then configure the
fetch agent/lookup or request target to use that validated address without
resolving the hostname again, preserving the existing validation failure
responses.
- Around line 10-16: Add the 0.0.0.0/8 network to PRIVATE_IP_RANGES, preserving
the existing range representation and ordering conventions so unspecified or
local-host addresses are treated as private.
- Line 134: Update the hostname handling in the URL validation flow to remove
the surrounding brackets from IPv6 literals before passing the value to
net.isIP() or DNS checks. Preserve unbracketed hostnames and non-IPv6 behavior,
so valid public IPv6 URLs bypass DNS resolution and are validated normally.
- Around line 196-235: Update createSafeFetch to track the number of redirects
followed and stop with an error once the configured maximum is reached, using a
standard cap such as 20. Propagate the incremented redirect count through the
recursive redirect call while preserving URL validation and normal response
handling.
- Around line 18-23: Fix ipv6ToBytes so a compressed “::” expands to the correct
number of zero hextets and does not advance the output index for each empty
split segment; verify leading, trailing, and embedded compression parses into
the exact 16-byte address. Extend IPv6 private-address validation to recognize
IPv4-mapped forms such as ::ffff:169.254.169.254 and ::ffff:127.0.0.1, either by
adding the mapped private range or routing the embedded IPv4 value through the
existing IPv4 private-range logic.
In `@agent/url-validator.test.js`:
- Around line 1-2: Update the dns/promises mock used by the URL validator tests
to create a hoisted resolver shared with the test lifecycle, expose it through
the mock’s default export with resolve, and reset it in beforeEach so
DNS-specific return values do not leak into other tests. Anchor the changes to
the dns/promises vi.mock setup and existing beforeEach/afterEach hooks while
preserving the current URL assertions.
In `@backend/openapi.json`:
- Around line 1975-1976: The OpenAPI document duplicates the policy path,
causing the later entry to overwrite GET, while the test list also masks the
issue. In backend/openapi.json lines 1975-1976, remove the duplicate path item
and merge its put operation into the existing /api/agents/{address}/policy item
containing get; in backend/test/openapi.test.js lines 44-59, remove the
duplicate requiredPaths entry and assert that this path exposes both get and put
operations.
- Around line 1262-1273: Move every operation-level X-Idempotency-Key entry from
the invalid headers arrays into each operation’s existing parameters array,
adding "in": "header" while preserving required, description, and schema
constraints. Apply this consistently to all five referenced operations and
remove the unsupported headers fields.
In `@backend/test/openapi.test.js`:
- Line 12: Update the SwaggerParser.validate call in the test setup to resolve
openapi.json relative to the test module URL rather than process.cwd(). Use the
module URL with the appropriate path-to-filesystem conversion so beforeAll loads
the package’s OpenAPI specification reliably.
---
Nitpick comments:
In `@agent/agent.js`:
- Around line 195-209: Remove the manual validateEndpointUrl call and its
duplicate SSRF_BLOCKED warning/error handling from the httpClient.fetch wrapper.
Rely on safeFetch, created via createSafeFetch, to perform endpoint validation
before each request, while preserving the existing request flow and error
propagation.
In `@backend/src/index.js`:
- Around line 93-106: Update the startup initialization and the /openapi.json
handler to load ../openapi.json once after confirming the runtime meets the
required Node version, cache its default export, and serve the cached spec
synchronously. Remove the per-request dynamic import and its associated runtime
exception handling while preserving the existing JSON response behavior.
In `@backend/test/openapi.test.js`:
- Around line 124-177: Replace the property-presence checks in the Response
Schema Validation tests with actual validation of each response.body against its
corresponding resolved OpenAPI component schema. Configure Ajv for OpenAPI 3.1’s
JSON Schema 2020-12 dialect, compile HealthResponse, ServicesListResponse,
AgentsListResponse, RegistryStats, and Error schemas, and assert validation
succeeds while preserving the existing status-code and conditional agents
behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4045c786-c9e7-4f22-a63a-ffa2c7bd6932
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
agent/README.mdagent/agent.jsagent/agent.test.jsagent/url-validator.jsagent/url-validator.test.jsbackend/openapi.jsonbackend/package.jsonbackend/src/index.jsbackend/test/openapi.test.js
| [EVENT.AGENT_COMPLETE]: { | ||
| required: ['event', 'agentAddress', 'totalTasks', 'successCount', 'failCount', 'totalUsdcSpent', 'runDurationMs'], | ||
| optional: ['finalScore', 'scoreDelta'], | ||
| types: { event: 'string', agentAddress: 'string', totalTasks: 'number', successCount: 'number', failCount: 'number', totalUsdcSpent: 'string', runDurationMs: 'number', finalScore: 'number', scoreDelta: 'number' }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)agent/(agent\.(js|test\.js)|README\.md)$' || true
echo "== relevant snippets =="
for f in agent/agent.js agent/agent.test.js agent/README.md; do
if [ -f "$f" ]; then
echo "--- $f ($(wc -l < "$f") lines) ---"
fi
done
echo "--- agent/agent.js outline/score mentions ---"
rg -n "scoreDelta|finalScore|score|AGENT_COMPLETE|eventAddress|agentAddress" agent/agent.js || true
echo "--- agent/agent.test.js lines 560-610 ---"
sed -n '560,610p' agent/agent.test.js || true
echo "--- agent/README.md lines 340-370 ---"
sed -n '340,370p' agent/README.md || true
echo "== behavioral parse contract evidence =="
python3 - <<'PY'
from pathlib import Path
import re
p = Path('agent/agent.test.js')
s = p.read_text()
needle = r'EVENT\.AGENT_COMPLETE:[\s\S]*?types:\s*{[^}]*}'
m = re.search(needle, s)
print(m.group(0) if m else "AGENT_COMPLETE schema not found")
# Extract declared optional keys/types if possible from the printed schema.
for field in ['finalScore','scoreDelta']:
print(field, "required:", "required:", bool(re.search(r"required:\s*\[[^\]]*\b"+re.escape(field)+r"\b", s)), "optional:", bool(re.search(r"optional:\s*\[[^\]]*\b"+re.escape(field)+r"\b", s)), "types:", "string" in re.search(r"types:\s*{[^}]*}", s).group(0) if re.search(r"types:\s*{[^}]*}", s) else False)
PYRepository: Stellar-Ecosystem/lodestar
Length of output: 6244
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- agent.js lines 380-475 ---"
sed -n '380,475p' agent/agent.js
echo "--- agent.test.js validation helper lines 610-650 ---"
sed -n '610,650p' agent/agent.test.js
echo "--- precise occurrences of finalScore/scoreDelta/null checks (docs/tests/code) ---"
rg -n "finalScore|scoreDelta|scoreBefore|scoreAfter|scoreAfterRegistration|scoringEnabled" agent/agent.js agent/agent.test.js agent/README.md
echo "== emit null absent semantics probe =="
python3 - <<'PY'
from pathlib import Path
p = Path('agent/agent.test.js')
text = p.read_text()
m_start = text.index('const schemas = {')
m_end = text.index('};', m_start)
schema_text = text[m_start:m_end+1]
assert 'finalScore' in schema_text and 'scoreDelta' in schema_text
types = 'types: {'
start = schema_text.index(types)
types = schema_text[start:schema_text.index('}', start)]
print('AGENT_COMPLETE optional includes both:', "optional: ['finalScore', 'scoreDelta']" in schema_text)
print('types contains finalScore number:', 'finalScore: \'number\'' in types)
print('types contains scoreDelta number:', 'scoreDelta: \'number\'' in types)
schema_block = schema_text[start:]
print('no null type union in schema text:', 'null' not in schema_block)
# Inspect implementation around AGENT_COMPLETE construction for whether fields exist when score is null.
src = Path('agent/agent.js').read_text()
idx = src.index('const finalScore = currentScore;')
block = src[idx:idx+350]
print('implementation always includes finalScore key:', 'finalScore,' in block)
print('implementation always includes scoreDelta key:', 'scoreDelta,' in block)
PYRepository: Stellar-Ecosystem/lodestar
Length of output: 9815
🌐 Web query:
Stellar Ecosystem Agent credit contract score null disable scoring
💡 Result:
There is no single "Stellar Ecosystem Agent" credit contract with a standardized "null disable" scoring function, as credit scoring on the Stellar network is implemented through various independent, custom Soroban smart contracts [1][2][3][4]. Because these are decentralized applications, the mechanisms for handling scores, resetting them, or disabling them depend entirely on the specific implementation of the contract you are using [1][5][6]. In the context of Soroban smart contracts on Stellar: 1. Absence of a Standardized API: Scoring logic is not a native network protocol feature but a contract-level implementation [1][2][4]. You must refer to the specific documentation or source code of the contract involved [1][6]. 2. Handling "Null" or "Disabled" Scores: - Many implementations treat a "null" or missing state as a baseline or default value (e.g., a neutral score of 40 or 500) rather than an error state [7][2]. - If a contract provides a way to "disable" scoring, it is typically handled via administrative functions (e.g., deregister_feeder or reset_project) or role-based access control where an admin can revoke a status [1][5][4]. - If you are attempting to programmatically handle an account with no recorded score, common practices include checking for the presence of the record in the contract's data storage before attempting a calculation [7][5]. 3. Security Considerations: - Always ensure that score modifications (like recording payments or resetting status) are protected by proper authorization checks (require_auth()) [5][3][6]. Improperly implemented contracts can allow unauthorized entities to manipulate scores [5]. To resolve your specific issue, you should: - Identify the specific repository or contract address you are interacting with [6]. - Review the lib.rs or contract.rs file within that project to see how the ScoreRecord or equivalent data structure is defined and if there is an explicit disable or reset function provided for administrators [1][5][4]. If you are developing your own contract and want to implement a "disable" feature, you should define a status flag (e.g., enum Status { Active, Disabled }) within your storage struct and add an administrative function that flips this flag, causing subsequent get_score calls to return a null or default value [1][4].
Citations:
- 1: https://github.com/cybermax4200/stellar-did-credit/blob/main/README.md
- 2: https://github.com/aasss18jul-dev/credit-scoring-system
- 3: https://github.com/blockful-io/trustful-stellar-v1
- 4: https://github.com/forcast-lmtd/dob-stellar
- 5: fix: credit_score record_payment has no authorization check — any caller can record payments astera-hq/Astera#637
- 6: https://github.com/chinesepowered/hack-stellaragents?tab=readme-ov-file
- 7: [CONTRACT] Handle credit score edge cases and tier boundaries Akanimoh12/Stellar-Tipz#12
Align optional score fields with null emission.
AGENT_COMPLETE always emits finalScore and scoreDelta, and the README example shows them as null when scoring is unavailable. The test schema rejects those nullable values as non-numbers, so the disabled-scoring path validates incorrectly. Add a disabled-scoring exercise to agent/agent.test.js and update finalScore/scoreDelta in the AGENT_COMPLETE schema and API docs to allow number | null.
📍 Affects 2 files
agent/agent.test.js#L590-L593(this comment)agent/README.md#L355-L360
🤖 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 `@agent/agent.test.js` around lines 590 - 593, Update the AGENT_COMPLETE schema
in agent/agent.test.js so finalScore and scoreDelta accept number or null, and
add a disabled-scoring test exercising the emitted null values. Update the
corresponding AGENT_COMPLETE example and field documentation in agent/README.md
to describe these fields as number | null.
| const validateEvent = (fields, eventName) => { | ||
| const schema = schemas[eventName]; | ||
| if (!schema) { | ||
| throw new Error(`No schema defined for event: ${eventName}`); | ||
| } | ||
|
|
||
| // Check required fields | ||
| for (const field of schema.required) { | ||
| if (!(field in fields)) { | ||
| throw new Error(`Event ${eventName} missing required field: ${field}`); | ||
| } | ||
| } | ||
|
|
||
| // Check field types | ||
| for (const [field, expectedType] of Object.entries(schema.types)) { | ||
| if (field in fields) { | ||
| const actualType = typeof fields[field]; | ||
| if (actualType !== expectedType) { | ||
| throw new Error(`Event ${eventName} field ${field} has type ${actualType}, expected ${expectedType}`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Ensure no unexpected fields (optional: can be strict or lenient) | ||
| // For now, we allow extra fields for flexibility | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Enforce the documented allowed-field set.
schema.optional is never used, and extra fields are explicitly allowed. Consequently, an undocumented field—or a field omitted from both required and types—will not fail this contract test. Validate each payload against required ∪ optional and explicitly handle any permitted logger metadata.
🤖 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 `@agent/agent.test.js` around lines 601 - 626, Update validateEvent to enforce
the schema’s allowed fields by combining schema.required and schema.optional,
rejecting payload fields outside that set. Preserve required-field and type
validation, and explicitly include the documented logger metadata fields in the
permitted set where applicable.
| ```javascript | ||
| import pino from 'pino'; | ||
| import { EVENT } from './agent.js'; | ||
|
|
||
| // Create a pino instance that reads from a log file or stream | ||
| const logger = pino({ | ||
| level: 'info', | ||
| transport: { target: 'pino-pretty', options: { colorize: true } }, | ||
| }); | ||
|
|
||
| // Or read from an existing log file: | ||
| // const logStream = fs.createReadStream('./agent.log'); | ||
| // const logger = pino(logStream); | ||
|
|
||
| // Process logs | ||
| logger.stream.on('data', (line) => { | ||
| try { | ||
| const parsed = JSON.parse(line); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a 'package.json' -x rg -n '"pino"|"pino-pretty"' {}
rg -n 'logger\.stream|pino-pretty|JSON\.parse|createReadStream' agent/README.mdRepository: Stellar-Ecosystem/lodestar
Length of output: 278
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "README context:"
sed -n '300,360p' agent/README.md || true
echo
echo "Search agent docs for pino/pretty/log examples:"
rg -n "pino|pino-pretty|logger\.stream|createReadStream|readline|JSON\.parse" agent/README.md || true
echo
echo "package files mentioning pino:"
fd -a 'package.json' -x sh -c 'echo "--- $1"; rg -n "\"pino\"|\"pino-pretty\"" "$1" || true' sh {}Repository: Stellar-Ecosystem/lodestar
Length of output: 2652
🌐 Web query:
Pino API logger stream pino-pretty format readable stream
💡 Result:
To format Pino logs into a readable stream using pino-pretty, you have two primary approaches: using the modern Transport API or using the legacy stream-based approach. The Transport API is the recommended method for most applications. 1. Transport API (Recommended) This method runs the prettifier in a separate worker thread, which is more performant and follows modern Pino best practices. const pino = require('pino'); const logger = pino({ transport: { target: 'pino-pretty', options: { colorize: true // Add other pino-pretty options here } } }); logger.info('This will be pretty-printed'); 2. Stream-based Approach This approach processes logs within the main thread. While still functional, it is generally intended for development environments. const pino = require('pino'); const pretty = require('pino-pretty'); // Create a readable stream const stream = pretty({ colorize: true }); const logger = pino(stream); logger.info('This will also be pretty-printed'); Advanced Optimization: Metadata Wrapper For improved performance when using the stream-based approach, you can use the asMetaWrapper method. This allows pino-pretty to work directly with Pino's log objects, skipping expensive JSON parsing. const pino = require('pino'); const pretty = require('pino-pretty'); const prettyStream = pretty({ colorize: true }); // Use asMetaWrapper to accept the writable stream const dest = prettyStream.asMetaWrapper(process.stdout); const logger = pino({}, dest); Key Considerations - Performance: Using pino-pretty in production is generally discouraged due to the overhead of prettifying logs [1][2]. - Flush Limitations: The logger.flush method is not supported when using pino-pretty [2][3]. - Development Setup: A common pattern is to only enable the transport when running in a TTY environment: const transport = process.stdout.isTTY? { transport: { target: 'pino-pretty' } }: {}; const logger = pino({...transport });
Citations:
- 1: https://www.npmjs.com/package/pino-pretty
- 2: https://raw.githubusercontent.com/pinojs/pino/main/docs/pretty.md
- 3: https://github.com/pinojs/pino/blob/HEAD/docs/pretty.md
Use a raw line reader for the JSON log example.
pino { transport: { target: 'pino-pretty' } } does not attach Pino’s readable destination to logger.stream; pino-pretty formats output instead. This sample can throw when accessing logger.stream, and JSON.parse(line) cannot parse formatted output.
Proposed consumer shape
-import pino from 'pino';
+import fs from 'node:fs';
+import readline from 'node:readline';
-const logger = pino({
- level: 'info',
- transport: { target: 'pino-pretty', options: { colorize: true } },
-});
+const input = fs.createReadStream('./agent.log', { encoding: 'utf8' });
+const lines = readline.createInterface({ input, crlfDelay: Infinity });
-logger.stream.on('data', (line) => {
+for await (const line of lines) {
const parsed = JSON.parse(line);
// process parsed event
-});
+}📝 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.
| ```javascript | |
| import pino from 'pino'; | |
| import { EVENT } from './agent.js'; | |
| // Create a pino instance that reads from a log file or stream | |
| const logger = pino({ | |
| level: 'info', | |
| transport: { target: 'pino-pretty', options: { colorize: true } }, | |
| }); | |
| // Or read from an existing log file: | |
| // const logStream = fs.createReadStream('./agent.log'); | |
| // const logger = pino(logStream); | |
| // Process logs | |
| logger.stream.on('data', (line) => { | |
| try { | |
| const parsed = JSON.parse(line); |
🤖 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 `@agent/README.md` around lines 323 - 340, Update the README JSON log example
to consume raw log lines from an actual readable stream or Pino destination
instead of accessing logger.stream from the pino-pretty transport. Ensure the
input remains newline-delimited JSON before passing each line to JSON.parse,
while keeping the existing log-processing flow intact.
| const PRIVATE_IP_RANGES = [ | ||
| { start: '10.0.0.0', prefix: 8 }, | ||
| { start: '172.16.0.0', prefix: 12 }, | ||
| { start: '192.168.0.0', prefix: 16 }, | ||
| { start: '169.254.0.0', prefix: 16 }, // Link-local | ||
| { start: '127.0.0.0', prefix: 8 }, // Loopback | ||
| ]; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Missing 0.0.0.0/8 in PRIVATE_IP_RANGES.
0.0.0.0 (and the wider 0.0.0.0/8 range) commonly routes to the local host on Linux/macOS and is a well-known SSRF bypass target, but it's absent from this list.
🛡️ Proposed fix
const PRIVATE_IP_RANGES = [
{ start: '10.0.0.0', prefix: 8 },
{ start: '172.16.0.0', prefix: 12 },
{ start: '192.168.0.0', prefix: 16 },
{ start: '169.254.0.0', prefix: 16 }, // Link-local
{ start: '127.0.0.0', prefix: 8 }, // Loopback
+ { start: '0.0.0.0', prefix: 8 }, // Unspecified / "this network"
];📝 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.
| const PRIVATE_IP_RANGES = [ | |
| { start: '10.0.0.0', prefix: 8 }, | |
| { start: '172.16.0.0', prefix: 12 }, | |
| { start: '192.168.0.0', prefix: 16 }, | |
| { start: '169.254.0.0', prefix: 16 }, // Link-local | |
| { start: '127.0.0.0', prefix: 8 }, // Loopback | |
| ]; | |
| const PRIVATE_IP_RANGES = [ | |
| { start: '10.0.0.0', prefix: 8 }, | |
| { start: '172.16.0.0', prefix: 12 }, | |
| { start: '192.168.0.0', prefix: 16 }, | |
| { start: '169.254.0.0', prefix: 16 }, // Link-local | |
| { start: '127.0.0.0', prefix: 8 }, // Loopback | |
| { start: '0.0.0.0', prefix: 8 }, // Unspecified / "this network" | |
| ]; |
🤖 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 `@agent/url-validator.js` around lines 10 - 16, Add the 0.0.0.0/8 network to
PRIVATE_IP_RANGES, preserving the existing range representation and ordering
conventions so unspecified or local-host addresses are treated as private.
| // IPv6 private ranges | ||
| const PRIVATE_IPV6_RANGES = [ | ||
| { start: '::1', prefix: 128 }, // Loopback | ||
| { start: 'fe80::', prefix: 10 }, // Link-local | ||
| { start: 'fc00::', prefix: 7 }, // Unique local | ||
| ]; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)url-validator\.js$|(^|/)url-validator\.(js|ts|mjs|cjs)$' || true
echo
echo "url-validator outline/size if present:"
if [ -f agent/url-validator.js ]; then
wc -l agent/url-validator.js
ast-grep outline agent/url-validator.js || true
echo
echo "Relevant sections:"
cat -n agent/url-validator.js | sed -n '1,140p'
fi
echo
echo "Search isPrivateIP/PRIVATE/ipv6ToBytes usages:"
rg -n "isPrivateIP|PRIVATE_IPV6_RANGES|ipv6ToBytes|\.isIP|isIP" . --glob '!node_modules' --glob '!dist' --glob '!build' || trueRepository: Stellar-Ecosystem/lodestar
Length of output: 6134
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe for the parsing bug / edge case impact without executing repository code.
node - <<'JS'
function ipv6ToBytesOriginal(ip) {
const bytes = new Uint8Array(16);
const parts = ip.split(':');
let byteIndex = 0;
for (let i = 0; i < parts.length; i++) {
const part = parts[i];
if (part === '') {
const skip = 8 - parts.length + 1;
byteIndex += skip * 2;
} else {
const num = parseInt(part, 16);
bytes[byteIndex++] = (num >> 8) & 0xFF;
bytes[byteIndex++] = num & 0xFF;
}
}
return bytes;
}
function ipv6ToBytesFixed(ip) {
const bytes = new Uint8Array(16);
const [head, tail] = ip.split('::');
const headParts = head ? head.split(':') : [];
const tailParts = tail ? head.split(':') : [];
const missing = 8 - headParts.length - tailParts.length;
const groups = [...headParts, ...Array(Math.max(missing, 0)).fill('0'), ...tailParts];
let byteIndex = 0;
for (const part of groups) {
const num = parseInt(part, 16);
bytes[byteIndex++] = (num >> 8) & 0xFF;
bytes[byteIndex++] = num & 0xFF;
}
return bytes;
}
const addrs = ['::1', '::ffff:127.0.0.1', '::ffff:169.254.169.254', 'fe80::1', 'fc00::1', '::'];
const net = require('net');
for (const ip of addrs) {
const bOrig = Array.from(ipv6ToBytesOriginal(ip)).join('');
const bNet = Buffer.from(ip, 'hex').toString('utf-8');
console.log(ip, 'original=' + bOrig, 'net.isIP=' + net.isIP(ip), 'net.compare=' + Buffer.compare(bOrig, Buffer.from(ip, 'hex')));
}
JS
node - <<'JS'
function isPrivateIP(ip) {
const PRIVATE_IPV4_RANGES = [
{ start: '10.0.0.0', end: '10.255.255.255' },
{ start: '172.16.0.0', end: '172.31.255.255' },
{ start: '192.168.0.0', end: '192.168.255.255' },
{ start: '127.0.0.0', end: '127.255.255.255' },
{ start: '169.254.0.0', end: '169.254.255.255' },
];
const PRIVATE_IPV6_RANGES = [
{ start: '::1', prefix: 128 }, // Loopback
{ start: 'fe80::', prefix: 10 }, // Link-local
{ start: 'fc00::', prefix: 7 }, // Unique local
];
const net = require('net');
if (!net.isIP(ip)) return false;
if (net.isIPv4(ip)) {
const parts = ip.split('.').map(Number);
for (const range of PRIVATE_IPV4_RANGES) {
const rangeStart = range.start.split('.').map(Number);
const rangeEnd = range.end.split('.').map(Number);
if (
parts[0] >= rangeStart[0] && parts[0] <= rangeEnd[0] &&
parts[1] >= rangeStart[1] && parts[1] <= rangeEnd[1] &&
parts[2] >= rangeStart[2] && parts[2] <= rangeEnd[2] &&
parts[3] >= rangeStart[3] && parts[3] <= rangeEnd[3]
) {
return true;
}
}
return false;
}
// IPv4-mapped IPv6 -> delegate to IPv4 checks
if (ip.startsWith('::ffff:') || ip.startsWith('::ffff:0:0/') || ip.includes(':')) {
const ipv4Match = ip.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)\/?(\d*)/i);
if (ipv4Match) {
const ipv4 = ipv4Match[1];
return isPrivateIP(ipv4);
}
}
if (!net.isIPv6(ip)) return false;
const bytes = ipv6ToBytes(ip);
const prefix = ipv6ToBytes(ip.replace('::', '0:'));
for (const range of PRIVATE_IPV6_RANGES) {
const rangeStart = ipv6ToBytes(range.start.replace('::', '0:'));
let bit = 0;
for (let i = 0; bit < range.prefix; i++) {
const bits = rangeStart[Math.floor(i / 8)];
const shifts = i % 8;
const bitValue = (bits >> (8 - shifts)) & 128;
const ourBits = bytes[Math.floor(i / 8)];
const byteValue = (ourBits >> (8 - shifts)) & 128;
if (bitValue !== byteValue) return false;
bit++;
}
}
return true;
}
function ipv6ToBytes(ip) {
const bytes = new Uint8Array(16);
const parts = ip.split(':');
let byteIndex = 0;
for (let i = 0; i < parts.length; i++) {
const part = parts[i];
if (part === '') {
const skip = 8 - parts.length + 1;
byteIndex += skip * 2;
} else {
const num = parseInt(part, 16);
bytes[byteIndex++] = (num >> 8) & 0xFF;
bytes[byteIndex++] = num & 0xFF;
}
}
return bytes;
}
for (const ip of ['::1', '::ffff:127.0.0.1', '::ffff:169.254.169.254', 'fe80::1', 'fc00::1', '::']) {
console.log(ip, isPrivateIP(ip));
}
JSRepository: Stellar-Ecosystem/lodestar
Length of output: 949
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Parse remainder:"
cat -n agent/url-validator.js | sed -n '135,235p'
echo
echo "Behavioral probe: ipv6ToBytes original and parsing behavior"
node - <<'JS'
function ipv6ToBytesOriginal(ip) {
const bytes = new Uint8Array(16);
const parts = ip.split(':');
let byteIndex = 0;
for (let i = 0; i < parts.length; i++) {
const part = parts[i];
if (part === '') {
const skip = 8 - parts.length + 1;
byteIndex += skip * 2;
} else {
const num = parseInt(part, 16);
bytes[byteIndex++] = (num >> 8) & 0xFF;
bytes[byteIndex++] = num & 0xFF;
}
}
return bytes;
}
for (const ip of ['::1', '::ffff:127.0.0.1', '::ffff:169.254.169.254', 'fe80::1', 'fc00::1', '::', '::1:2:3:4:5:6:7:8']) {
const parts = ip.split(':');
let ok = true;
try {
const bytes = Array.from(ipv6ToBytesOriginal(ip));
const netHex = Buffer.from(ip, 'hex');
console.log(JSON.stringify({... {
ip,
splitLength: parts.length,
byteIndexEnd: parts.length > 0 && parts[0] === '' ? undefined : undefined,
parseHex: bytes.join(''),
netHex: netHex.toString(),
equal: Buffer.compare(Uint8Array.from(bytes), Uint8Array.from(netHex)) === 0,
}}));
} catch (e) {
console.log(JSON.stringify({... {ip, splitLength: parts.length, error: e && e.message, truncated: ok && false}}));
}
}
JS
echo
echo "Behavioral probe: current isPrivateIP with original ipv6ToBytes"
node - <<'JS'
function isPrivateIP(ip) {
const PRIVATE_IP_RANGES = [
{ start: '10.0.0.0', prefix: 8 },
{ start: '172.16.0.0', prefix: 12 },
{ start: '192.168.0.0', prefix: 16 },
{ start: '169.254.0.0', prefix: 16 },
{ start: '127.0.0.0', prefix: 8 },
];
const PRIVATE_IPV6_RANGES = [
{ start: '::1', prefix: 128 },
{ start: 'fe80::', prefix: 10 },
{ start: 'fc00::', prefix: 7 },
];
const net = require('net');
if (!net.isIP(ip)) return false;
if (net.isIPv4(ip)) {
const parts = ip.split('.').map(Number);
for (const range of PRIVATE_IP_RANGES) {
const rangeStart = range.start.split('.').map(Number);
const rangeEnd = range.end.split('.').map(Number);
if (
parts[0] >= rangeStart[0] && parts[0] <= rangeEnd[0] &&
parts[1] >= rangeStart[1] && parts[1] <= rangeEnd[1] &&
parts[2] >= rangeStart[2] && parts[2] <= rangeEnd[2] &&
parts[3] >= rangeStart[3] && parts[3] <= rangeEnd[3]
) {
return true;
}
}
return false;
}
if (net.isIPv6(ip)) {
for (const range of PRIVATE_IPV6_RANGES) {
if (isIPv6InRange(ip, range.start, range.prefix)) return true;
}
}
return false;
}
function isIPv6InRange(ip, start, prefix) {
const ipBytes = ipv6ToBytes(ip);
const startBytes = ipv6ToBytes(start);
const maskBytes = prefixToMask(prefix);
for (let i = 0; i < 16; i++) {
if ((ipBytes[i] & maskBytes[i]) !== (startBytes[i] & maskBytes[i])) return false;
}
return true;
}
function ipv6ToBytes(ip) {
const bytes = new Uint8Array(16);
const parts = ip.split(':');
let byteIndex = 0;
for (let i = 0; i < parts.length; i++) {
const part = parts[i];
if (part === '') {
const skip = 8 - parts.length + 1;
byteIndex += skip * 2;
} else {
const num = parseInt(part, 16);
bytes[byteIndex++] = (num >> 8) & 0xFF;
bytes[byteIndex++] = num & 0xFF;
}
}
return bytes;
}
function prefixToMask(prefix) {
const bytes = new Uint8Array(16);
for (let i = 0; i < 16; i++) {
const bitsRemaining = Math.max(0, prefix - i * 8);
bytes[i] = bitsRemaining >= 8 ? 0xFF : (0xFF << (8 - bitsRemaining)) & 0xFF;
}
return bytes;
}
for (const ip of [
'::1',
'::1:2:3:4:5:6:7:8',
'::ffff:127.0.0.1',
'::ffff:169.254.169.254',
'fe80::1',
'fc00::1',
'::'
]) {
console.log(ip, isPrivateIP(ip));
}
JSRepository: Stellar-Ecosystem/lodestar
Length of output: 5028
Fix IPv6 :: parsing so parsed bytes match the address.
ipv6ToBytes only works for the current loopback-vs-loopback check by accident. Leading/trailing :: addresses like ::ffff:169.254.169.254, ::ffff:127.0.0.1, fe80::1, and fc00::1 can be mis-parsed because each empty segment from split(':') advances the write index multiple times and truncates at the 16-byte array. Once :: parsing is fixed, also add the IPv4-mapped private range check or resolve ::ffff:<IPv4> through the IPv4 private-range logic.
🤖 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 `@agent/url-validator.js` around lines 18 - 23, Fix ipv6ToBytes so a compressed
“::” expands to the correct number of zero hextets and does not advance the
output index for each empty split segment; verify leading, trailing, and
embedded compression parses into the exact 16-byte address. Extend IPv6
private-address validation to recognize IPv4-mapped forms such as
::ffff:169.254.169.254 and ::ffff:127.0.0.1, either by adding the mapped private
range or routing the embedded IPv4 value through the existing IPv4 private-range
logic.
| export function createSafeFetch(baseFetch) { | ||
| return async (url, options = {}) => { | ||
| // Validate initial URL | ||
| const validation = await validateEndpointUrl(url); | ||
| if (!validation.valid) { | ||
| const err = new Error(validation.reason); | ||
| err.code = 'SSRF_BLOCKED'; | ||
| throw err; | ||
| } | ||
|
|
||
| // Create fetch with redirect handling | ||
| const response = await baseFetch(url, { | ||
| ...options, | ||
| redirect: 'manual', // We'll handle redirects manually | ||
| }); | ||
|
|
||
| // Handle redirects | ||
| if (response.status >= 300 && response.status < 400) { | ||
| const location = response.headers.get('location'); | ||
| if (!location) { | ||
| return response; | ||
| } | ||
|
|
||
| // Resolve relative URLs | ||
| const redirectUrl = new URL(location, url).toString(); | ||
|
|
||
| const redirectValidation = await validateRedirect(url, redirectUrl); | ||
| if (!redirectValidation.valid) { | ||
| const err = new Error(redirectValidation.reason); | ||
| err.code = 'SSRF_REDIRECT_BLOCKED'; | ||
| throw err; | ||
| } | ||
|
|
||
| // Follow the redirect | ||
| return createSafeFetch(baseFetch)(redirectUrl, options); | ||
| } | ||
|
|
||
| return response; | ||
| }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
No cap on redirect-following in createSafeFetch.
Each valid redirect recurses into createSafeFetch(baseFetch)(redirectUrl, options) with no maximum-redirect counter. A malicious or misconfigured allowlisted/same-host server that keeps issuing 3xx responses (even a simple redirect loop) causes indefinite request chains instead of failing fast, unlike standard fetch implementations which cap redirects (commonly 20).
🛡️ Proposed fix
-export function createSafeFetch(baseFetch) {
- return async (url, options = {}) => {
+export function createSafeFetch(baseFetch, maxRedirects = 5) {
+ return async (url, options = {}, redirectCount = 0) => {
+ if (redirectCount > maxRedirects) {
+ const err = new Error(`Too many redirects for ${url}`);
+ err.code = 'SSRF_REDIRECT_BLOCKED';
+ throw err;
+ }
// Validate initial URL
const validation = await validateEndpointUrl(url);
...
- return createSafeFetch(baseFetch)(redirectUrl, options);
+ return createSafeFetch(baseFetch, maxRedirects)(redirectUrl, options, redirectCount + 1);📝 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.
| export function createSafeFetch(baseFetch) { | |
| return async (url, options = {}) => { | |
| // Validate initial URL | |
| const validation = await validateEndpointUrl(url); | |
| if (!validation.valid) { | |
| const err = new Error(validation.reason); | |
| err.code = 'SSRF_BLOCKED'; | |
| throw err; | |
| } | |
| // Create fetch with redirect handling | |
| const response = await baseFetch(url, { | |
| ...options, | |
| redirect: 'manual', // We'll handle redirects manually | |
| }); | |
| // Handle redirects | |
| if (response.status >= 300 && response.status < 400) { | |
| const location = response.headers.get('location'); | |
| if (!location) { | |
| return response; | |
| } | |
| // Resolve relative URLs | |
| const redirectUrl = new URL(location, url).toString(); | |
| const redirectValidation = await validateRedirect(url, redirectUrl); | |
| if (!redirectValidation.valid) { | |
| const err = new Error(redirectValidation.reason); | |
| err.code = 'SSRF_REDIRECT_BLOCKED'; | |
| throw err; | |
| } | |
| // Follow the redirect | |
| return createSafeFetch(baseFetch)(redirectUrl, options); | |
| } | |
| return response; | |
| }; | |
| } | |
| export function createSafeFetch(baseFetch, maxRedirects = 5) { | |
| return async (url, options = {}, redirectCount = 0) => { | |
| if (redirectCount > maxRedirects) { | |
| const err = new Error(`Too many redirects for ${url}`); | |
| err.code = 'SSRF_REDIRECT_BLOCKED'; | |
| throw err; | |
| } | |
| // Validate initial URL | |
| const validation = await validateEndpointUrl(url); | |
| if (!validation.valid) { | |
| const err = new Error(validation.reason); | |
| err.code = 'SSRF_BLOCKED'; | |
| throw err; | |
| } | |
| // Create fetch with redirect handling | |
| const response = await baseFetch(url, { | |
| ...options, | |
| redirect: 'manual', // We'll handle redirects manually | |
| }); | |
| // Handle redirects | |
| if (response.status >= 300 && response.status < 400) { | |
| const location = response.headers.get('location'); | |
| if (!location) { | |
| return response; | |
| } | |
| // Resolve relative URLs | |
| const redirectUrl = new URL(location, url).toString(); | |
| const redirectValidation = await validateRedirect(url, redirectUrl); | |
| if (!redirectValidation.valid) { | |
| const err = new Error(redirectValidation.reason); | |
| err.code = 'SSRF_REDIRECT_BLOCKED'; | |
| throw err; | |
| } | |
| // Follow the redirect | |
| return createSafeFetch(baseFetch, maxRedirects)(redirectUrl, options, redirectCount + 1); | |
| } | |
| return response; | |
| }; | |
| } |
🤖 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 `@agent/url-validator.js` around lines 196 - 235, Update createSafeFetch to
track the number of redirects followed and stop with an error once the
configured maximum is reached, using a standard cap such as 20. Propagate the
incremented redirect count through the recursive redirect call while preserving
URL validation and normal response handling.
| import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; | ||
| import { validateEndpointUrl, validateRedirect, createSafeFetch } from './url-validator.js'; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)agent/(url-validator\.(js|test\.js)|.*\.js)$' || true
echo "== test outline =="
wc -l agent/url-validator.test.js agent/url-validator.js 2>/dev/null || true
echo "== relevant test lines =="
sed -n '1,90p' agent/url-validator.test.js
echo "== dns mock area =="
sed -n '90,175p' agent/url-validator.test.js
echo "== later URL tests =="
sed -n '175,360p' agent/url-validator.test.js
echo "== validator import =="
sed -n '1,80p' agent/url-validator.js
echo "== vitest config/package =="
for f in package.json vitest.config.js vitest.config.ts vite.config.js vite.config.ts; do
[ -f "$f" ] && { echo "--- $f"; sed -n '1,220p' "$f"; }
doneRepository: Stellar-Ecosystem/lodestar
Length of output: 14467
🌐 Web query:
Vitest vi.mock factory default export required for default import namespace hoisting
💡 Result:
In Vitest, when you use a factory function to mock a module containing a default export, you must explicitly include a default key in the returned object [1][2][3]. This requirement arises because Vitest operates in an ES module (ESM) environment, where default exports are treated as a distinct property of the module object [2]. Unlike some legacy CommonJS-based testing tools that might handle default exports implicitly, Vitest requires this explicit definition to correctly map the export for ESM compliance [1][3]. When using vi.mock, the call is hoisted to the top of the file and executed before any static imports [1][4][5]. This hoisting ensures that the mock is registered before the module is actually imported [6][7]. Because of this hoisting, the code inside your factory function cannot reference variables defined in the outer scope of the file [1][8]. If you need to reference external variables within your mock factory (e.g., to create a spy or share state), you should define them using vi.hoisted [1][8]. Example of mocking a default export: vi.mock('./path/to/module.js', => { return { default: { myDefaultKey: vi.fn }, namedExport: vi.fn, } }) If you attempt to mock a module with a default export without providing the default key, the default import will likely result in undefined [2].
Citations:
- 1: https://vitest.dev/api/vi
- 2: Factory function returns undefined vitest-dev/vitest#1011
- 3: https://github.com/vitest-dev/vitest/blob/206e8cff/docs/api/vi.md
- 4: https://github.com/vitest-dev/vitest/blob/main/docs/guide/mocking.md
- 5: https://vitest.dev/guide/mocking
- 6: https://vitest.dev/guide/mocking/modules
- 7: https://github.com/vitest-dev/vitest/blob/1a4705da/docs/guide/mocking/modules.md
- 8: Introduce
vi.hoistedto run code before imports andvi.mockvitest-dev/vitest#3228
Hoist and reset the dns/promises mock with a default export.
agent/url-validator.js uses import dns from 'dns/promises', while the inline vi.mock('dns/promises', ...) factory only returns a named resolve export. Vitest hoists the factory too, so dns can become undefined instead of an object with resolve, and the hardcoded private/public DNS return values persist across tests because there is no hoisted mock shared with beforeEach/afterEach reset.
Use a hoisted resolver and include the default export, then reset it before each test so normal HTTPS URL assertions are not affected by previous DNS tests.
🤖 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 `@agent/url-validator.test.js` around lines 1 - 2, Update the dns/promises mock
used by the URL validator tests to create a hoisted resolver shared with the
test lifecycle, expose it through the mock’s default export with resolve, and
reset it in beforeEach so DNS-specific return values do not leak into other
tests. Anchor the changes to the dns/promises vi.mock setup and existing
beforeEach/afterEach hooks while preserving the current URL assertions.
| "headers": [ | ||
| { | ||
| "name": "X-Idempotency-Key", | ||
| "required": true, | ||
| "description": "Unique idempotency key for the payment (1-255 printable ASCII characters)", | ||
| "schema": { | ||
| "type": "string", | ||
| "minLength": 1, | ||
| "maxLength": 255 | ||
| } | ||
| } | ||
| ], |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
headers is not a valid Operation Object field — required auth headers won't be documented.
Operation-level headers arrays (also at Lines 1502-1512, 1848-1858, 1910-1920, 1992-2002) are ignored by OpenAPI tooling. Header inputs belong in parameters with "in": "header".
♻️ Proposed fix
- "headers": [
- {
- "name": "X-Idempotency-Key",
- "required": true,
- "description": "Unique idempotency key for the payment (1-255 printable ASCII characters)",
- "schema": {
- "type": "string",
- "minLength": 1,
- "maxLength": 255
- }
- }
- ],and add the entry to the existing parameters array:
{
"name": "X-Idempotency-Key",
"in": "header",
"required": true,
"description": "Unique idempotency key for the payment (1-255 printable ASCII characters)",
"schema": { "type": "string", "minLength": 1, "maxLength": 255 }
}🤖 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 `@backend/openapi.json` around lines 1262 - 1273, Move every operation-level
X-Idempotency-Key entry from the invalid headers arrays into each operation’s
existing parameters array, adding "in": "header" while preserving required,
description, and schema constraints. Apply this consistently to all five
referenced operations and remove the unsupported headers fields.
| "/api/agents/{address}/policy": { | ||
| "put": { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Duplicate /api/agents/{address}/policy path item, mirrored in the test list. The spec declares the same path key twice — Line 935 (get) and Line 1975 (put) — so the later JSON key wins and the GET operation with its AgentPolicy response disappears from the served document. The test's requiredPaths array lists that path twice as well, so the assertion passes regardless and cannot detect the loss.
backend/openapi.json#L1975-L1976: delete this second"/api/agents/{address}/policy"entry and move itsputoperation into the existing path item at Line 935 alongsideget.backend/test/openapi.test.js#L44-L59: remove the duplicate'/api/agents/{address}/policy'entry at Line 59 and add an assertion that the path item exposes bothgetandput.
📍 Affects 2 files
backend/openapi.json#L1975-L1976(this comment)backend/test/openapi.test.js#L44-L59
🤖 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 `@backend/openapi.json` around lines 1975 - 1976, The OpenAPI document
duplicates the policy path, causing the later entry to overwrite GET, while the
test list also masks the issue. In backend/openapi.json lines 1975-1976, remove
the duplicate path item and merge its put operation into the existing
/api/agents/{address}/policy item containing get; in
backend/test/openapi.test.js lines 44-59, remove the duplicate requiredPaths
entry and assert that this path exposes both get and put operations.
Source: Linters/SAST tools
|
|
||
| beforeAll(async () => { | ||
| api = request(app); | ||
| spec = await SwaggerParser.validate('../openapi.json'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Relative spec path is resolved against process.cwd(), not this file.
swagger-parser resolves a relative path from the working directory. With Vitest rooted at backend/, '../openapi.json' points outside the package and beforeAll will throw, failing every test in the suite. Resolve it from the module URL instead.
🐛 Proposed fix
+import { fileURLToPath } from 'node:url';
...
- spec = await SwaggerParser.validate('../openapi.json');
+ spec = await SwaggerParser.validate(
+ fileURLToPath(new URL('../openapi.json', import.meta.url))
+ );📝 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.
| spec = await SwaggerParser.validate('../openapi.json'); | |
| import { fileURLToPath } from 'node:url'; | |
| spec = await SwaggerParser.validate( | |
| fileURLToPath(new URL('../openapi.json', import.meta.url)) | |
| ); |
🤖 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 `@backend/test/openapi.test.js` at line 12, Update the SwaggerParser.validate
call in the test setup to resolve openapi.json relative to the test module URL
rather than process.cwd(). Use the module URL with the appropriate
path-to-filesystem conversion so beforeAll loads the package’s OpenAPI
specification reliably.
|
Hi @samueldoo, This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
2 participants
Closes #365
Summary
This PR establishes clear field schemas and contracts for all canonical events emitted by the agent (
agent/agent.js), making it possible to build reliable log consumers and telemetry integrations.Key Changes
Event Schema Documentation (
agent/README.md):EVENT:agent_startagent_registeredtask_startservice_selectedspend_check_passedspend_check_blockedpayment_successpayment_failedscore_updatedagent_completeAutomated Schema Validation (
agent/agent.test.js):Verification
npx vitest run agent.test.js— all 29 tests passed cleanly.Summary by CodeRabbit
Security
Documentation
/openapi.json.Testing