Add dashboard admin api #17
Conversation
📝 WalkthroughWalkthroughThe pull request introduces authenticated node information endpoints to the API. A new configuration field stores the admin secret, while a new route module implements three endpoints that retrieve node data with varying detail levels, protected by header-based authentication. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as API Server
participant Auth as Auth Check
participant DB as Node Collection
participant Formatter as Response
Client->>API: GET /nodes_info (x-api-secret header)
API->>Auth: checkAuth(req)
alt Secret Valid
Auth-->>API: ✓ Authenticated
API->>DB: getCollection().all()
DB-->>API: nodes array
API->>Formatter: Filter fields (name, node, coinbase, ip)
Formatter->>Formatter: Calculate stats (active, mining, peers, pending)
Formatter-->>Client: 200 JSON {filtered data, count, stats}
else Secret Invalid/Missing
Auth-->>API: ✗ Auth Failed
API-->>Client: 401 Unauthorized JSON
else Collection Error
DB-->>API: Exception
API-->>Client: 500 Internal Server Error JSON
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
🤖 Fix all issues with AI agents
In `@lib/routes/nodesinfo.js`:
- Around line 108-118: Replace the direct string comparison in checkAuth with a
timing-safe check: ensure both providedSecret and config.adminSecret exist,
convert them to Buffers (e.g., Buffer.from(..., 'utf8')), and use
crypto.timingSafeEqual to compare them while guarding against different lengths
(return false if buffer lengths differ) so timingSafeEqual is only called on
equal-length buffers; keep the existing early-false behavior when adminSecret or
providedSecret is missing.
- Around line 85-93: The verbose endpoint currently returns raw node objects
(from getCollection() -> Nodes.all()) including internal fields like spark;
change getNodesInfoVerbose to transform/map allNodes to only include the safe
API fields used by getNodesCoinbase/getNodesInfo (e.g., id, host, port, status,
coinbase, etc.) and explicitly omit internal/socket fields such as spark before
calling sendPrettyJson(res, 200, ...); follow the same field whitelist/shape
used in getNodesCoinbase/getNodesInfo to ensure consistent, serializable
responses.
🧹 Nitpick comments (3)
lib/routes/nodesinfo.js (2)
1-1: Remove unused import.The
getfunction is imported fromgruntbut never used in this file. This appears to be dead code.Suggested fix
-const { get } = require("grunt"); const { config } = require("../../config"); const { getCollection } = require("../collection");
5-5: Handlers are declaredasyncbut don't useawait.These handler functions are declared
asyncbut contain noawaitexpressions. SincegetCollection().all()appears synchronous, theasynckeyword is unnecessary. This is functionally harmless but slightly misleading.Also applies to: 36-36, 77-77
lib/routes/index.js (1)
13-15: Consider wrapping routes withasyncWrapperfor consistency.The new routes don't use
asyncWrapper, unlike the forensics routes (lines 17-20). While the handlers have internal try/catch blocks that should handle errors, wrapping them withasyncWrapperwould provide a consistent error-handling pattern and an additional safety net for any uncaught exceptions.Suggested change for consistency
-router.get('/nodes_info', getNodesInfo); -router.get('/nodes_info_verbose', getNodesInfoVerbose); -router.get('/coinbase_info', getNodesCoinbase); +router.get('/nodes_info', asyncWrapper(getNodesInfo)); +router.get('/nodes_info_verbose', asyncWrapper(getNodesInfoVerbose)); +router.get('/coinbase_info', asyncWrapper(getNodesCoinbase));
| try { | ||
| const Nodes = getCollection(); | ||
| const allNodes = Nodes.all(); | ||
|
|
||
| sendPrettyJson(res, 200, { | ||
| success: true, | ||
| count: allNodes.length, | ||
| nodes: allNodes, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check what fields exist on node objects
rg -n "node\." lib/collection.js -A2 -B2 | head -100Repository: XinFinOrg/XDCStats
Length of output: 1519
🏁 Script executed:
# Also check the full context of the getNodesInfoVerbose endpoint including auth
cat -n lib/routes/nodesinfo.js | head -100Repository: XinFinOrg/XDCStats
Length of output: 3288
🏁 Script executed:
# Look for node object creation/definition to understand its structure
rg -n "this\..*=.*node" lib/ -A1 | head -60Repository: XinFinOrg/XDCStats
Length of output: 44
🏁 Script executed:
# Find Node class definition
find lib -name "*.js" -type f | xargs grep -l "class Node\|function Node" | head -5Repository: XinFinOrg/XDCStats
Length of output: 44
🏁 Script executed:
# Search for node object property definitions
rg -n "this\.[a-zA-Z]+ =" lib/node.js -A1 | head -80Repository: XinFinOrg/XDCStats
Length of output: 720
🏁 Script executed:
# Also check if there are internal/socket references in nodes
rg -n "socket|connection|ws|WebSocket" lib/node.js | head -30Repository: XinFinOrg/XDCStats
Length of output: 44
🏁 Script executed:
# Check what 'spark' is in the context
rg -n "spark" lib/node.js -B2 -A2Repository: XinFinOrg/XDCStats
Length of output: 149
🏁 Script executed:
# Also check if spark is ever used elsewhere or initialized
rg -n "spark" lib/ -t js | head -20Repository: XinFinOrg/XDCStats
Length of output: 317
🏁 Script executed:
# Check what JSON.stringify or serialization might do with socket objects
rg -n "JSON.stringify|sendPrettyJson" lib/routes/nodesinfo.js -B2 -A2Repository: XinFinOrg/XDCStats
Length of output: 1890
🏁 Script executed:
# Check if Node class has a toJSON method for serialization
rg -n "toJSON" lib/node.jsRepository: XinFinOrg/XDCStats
Length of output: 44
🏁 Script executed:
# Check the full Node class to understand all properties
cat -n lib/node.js | head -120Repository: XinFinOrg/XDCStats
Length of output: 3024
🏁 Script executed:
# Check if spark is ever used/accessed in responses or if it's just stored but not used
rg -n "node.spark|\.spark" lib/routes/ -A2 -B2Repository: XinFinOrg/XDCStats
Length of output: 44
🏁 Script executed:
# Verify what JSON.stringify does with the spark connection object by checking if there are any serialization guards
rg -n "replacer|toJSON|stringify" lib/ -t js | head -20Repository: XinFinOrg/XDCStats
Length of output: 244
🏁 Script executed:
# Let's verify what spark actually is by checking the collection initialization
rg -n "spark" lib/collection.js -B5 -A5 | head -50Repository: XinFinOrg/XDCStats
Length of output: 599
Filter sensitive internal fields from the verbose endpoint response.
The getNodesInfoVerbose endpoint returns raw node objects including the spark field, which is a WebSocket connection object used internally for socket management. This is an implementation detail that shouldn't be exposed via the API and may cause serialization issues. Follow the pattern used in getNodesCoinbase and getNodesInfo by explicitly filtering the node objects to include only safe, API-intended fields before sending the response.
🤖 Prompt for AI Agents
In `@lib/routes/nodesinfo.js` around lines 85 - 93, The verbose endpoint currently
returns raw node objects (from getCollection() -> Nodes.all()) including
internal fields like spark; change getNodesInfoVerbose to transform/map allNodes
to only include the safe API fields used by getNodesCoinbase/getNodesInfo (e.g.,
id, host, port, status, coinbase, etc.) and explicitly omit internal/socket
fields such as spark before calling sendPrettyJson(res, 200, ...); follow the
same field whitelist/shape used in getNodesCoinbase/getNodesInfo to ensure
consistent, serializable responses.
| const checkAuth = (req) => { | ||
| const providedSecret = req.headers["x-api-secret"]; | ||
| if ( | ||
| !config.adminSecret || | ||
| !providedSecret || | ||
| providedSecret !== config.adminSecret | ||
| ) { | ||
| return false; | ||
| } | ||
| return true; | ||
| }; |
There was a problem hiding this comment.
Use timing-safe comparison for secret validation.
Direct string comparison (!==) is vulnerable to timing attacks. An attacker could potentially measure response times to infer the secret character by character. Use crypto.timingSafeEqual for constant-time comparison.
Suggested fix
+const crypto = require("crypto");
const { config } = require("../../config");
const { getCollection } = require("../collection"); const checkAuth = (req) => {
const providedSecret = req.headers["x-api-secret"];
- if (
- !config.adminSecret ||
- !providedSecret ||
- providedSecret !== config.adminSecret
- ) {
+ if (!config.adminSecret || !providedSecret) {
return false;
}
- return true;
+ try {
+ return crypto.timingSafeEqual(
+ Buffer.from(providedSecret),
+ Buffer.from(config.adminSecret)
+ );
+ } catch {
+ return false;
+ }
};📝 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 checkAuth = (req) => { | |
| const providedSecret = req.headers["x-api-secret"]; | |
| if ( | |
| !config.adminSecret || | |
| !providedSecret || | |
| providedSecret !== config.adminSecret | |
| ) { | |
| return false; | |
| } | |
| return true; | |
| }; | |
| const checkAuth = (req) => { | |
| const providedSecret = req.headers["x-api-secret"]; | |
| if (!config.adminSecret || !providedSecret) { | |
| return false; | |
| } | |
| try { | |
| return crypto.timingSafeEqual( | |
| Buffer.from(providedSecret), | |
| Buffer.from(config.adminSecret) | |
| ); | |
| } catch { | |
| return false; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In `@lib/routes/nodesinfo.js` around lines 108 - 118, Replace the direct string
comparison in checkAuth with a timing-safe check: ensure both providedSecret and
config.adminSecret exist, convert them to Buffers (e.g., Buffer.from(...,
'utf8')), and use crypto.timingSafeEqual to compare them while guarding against
different lengths (return false if buffer lengths differ) so timingSafeEqual is
only called on equal-length buffers; keep the existing early-false behavior when
adminSecret or providedSecret is missing.
Add admin APIs to gather nodes information in JSON format. The request must supply the correct ADMIN_SECRET in "x-api-secret" header.
/v2/nodes_info
/v2/nodes_info_verbose
/v2/coinbase_info
Summary by CodeRabbit
/nodes_info,/nodes_info_verbose, and/coinbase_info/forensics/load/latestfor retrieving the latest forensics report✏️ Tip: You can customize this high-level summary in your review settings.