Skip to content

Add querymongo passthrough query mode to GET /node#413

Open
wilke wants to merge 1 commit intomasterfrom
feature/mongo
Open

Add querymongo passthrough query mode to GET /node#413
wilke wants to merge 1 commit intomasterfrom
feature/mongo

Conversation

@wilke
Copy link
Member

@wilke wilke commented Mar 5, 2026

Summary

  • Adds ?querymongo=<JSON> parameter to GET /node enabling raw MongoDB queries ($in, $exists, $elemMatch, nested $and/$or, etc.)
  • Sanitizes queries to block dangerous operators ($where, $expr, $function, $accumulator) with recursive checking
  • Enforces 16KB JSON size limit to prevent abuse; permission filtering (qPerm) is always applied

Test plan

  • go build compiles without errors
  • go vet passes (no new warnings)
  • Unit tests for sanitizeQuery pass — covers safe operators, blocked operators at top level, and nested blocked operators
  • Run ./run-tests.sh all for full Docker integration tests

🤖 Generated with Claude Code

Add support for raw MongoDB queries via ?querymongo=<JSON> parameter,
enabling $in, $exists, $elemMatch, nested $and/$or, and other MongoDB
operators not available through existing query/querynode modes. Includes
sanitization to block dangerous operators ($where, $expr, $function,
$accumulator) and a 16KB size limit. Permission filtering (qPerm) is
always enforced.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new passthrough query mode to the GET /node endpoint that accepts raw MongoDB query JSON via ?querymongo=..., with guardrails to reduce security risk.

Changes:

  • Introduces querymongo query parameter parsing in NodeController.ReadMany, including JSON size limiting and operator sanitization.
  • Adds a recursive sanitizeQuery helper that blocks specific dangerous MongoDB operators.
  • Adds unit tests validating allowed vs. blocked operators for sanitizeQuery.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
shock-server/controller/node/multi.go Adds querymongo handling, size limit, and recursive operator sanitization.
shock-server/controller/node/multi_test.go Adds unit tests for sanitizeQuery safe/blocked operator behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +113 to +126
var userQuery bson.M
if err := json.Unmarshal([]byte(rawJSON), &userQuery); err != nil {
err_msg := "err@node_ReadMany: invalid JSON in querymongo parameter: " + err.Error()
logger.Error(err_msg)
responder.RespondWithError(w, r, http.StatusBadRequest, err_msg)
return
}
if err := sanitizeQuery(userQuery); err != nil {
err_msg := "err@node_ReadMany: " + err.Error()
logger.Error(err_msg)
responder.RespondWithError(w, r, http.StatusBadRequest, err_msg)
return
}
qOpts = userQuery
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

userQuery is declared as bson.M (a named map type), but sanitizeQuery expects map[string]interface{}. In Go, a named map type isn’t directly assignable to an unnamed map[...]..., so this call won’t compile. Fix by either changing sanitizeQuery to accept bson.M (and updating tests accordingly) or by explicitly converting at the call site (e.g., converting userQuery to map[string]interface{}) / making sanitizeQuery accept an interface{} and handle both map types.

Copilot uses AI. Check for mistakes.
Comment on lines +105 to 127
} else if queryJSON, ok := query["querymongo"]; ok {
rawJSON := queryJSON[0]
if len(rawJSON) > maxQueryJSONSize {
err_msg := fmt.Sprintf("err@node_ReadMany: querymongo JSON exceeds maximum size of %d bytes", maxQueryJSONSize)
logger.Error(err_msg)
responder.RespondWithError(w, r, http.StatusBadRequest, err_msg)
return
}
var userQuery bson.M
if err := json.Unmarshal([]byte(rawJSON), &userQuery); err != nil {
err_msg := "err@node_ReadMany: invalid JSON in querymongo parameter: " + err.Error()
logger.Error(err_msg)
responder.RespondWithError(w, r, http.StatusBadRequest, err_msg)
return
}
if err := sanitizeQuery(userQuery); err != nil {
err_msg := "err@node_ReadMany: " + err.Error()
logger.Error(err_msg)
responder.RespondWithError(w, r, http.StatusBadRequest, err_msg)
return
}
qOpts = userQuery
}
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

The new querymongo passthrough path in ReadMany has no controller-level tests to verify request validation behavior (e.g., invalid JSON returns 400, oversized JSON hits the 16KB limit) and that the handler short-circuits before DB access. Adding httptest-based coverage similar to node_controller_test.go would help prevent regressions for this new public API surface.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants