Skip to content

Fix N1QL injection in nested Hash attribute updates#33

Open
pimpin wants to merge 3 commits into
masterfrom
pimpin/fix-n1ql-injection-hash
Open

Fix N1QL injection in nested Hash attribute updates#33
pimpin wants to merge 3 commits into
masterfrom
pimpin/fix-n1ql-injection-hash

Conversation

@pimpin

@pimpin pimpin commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Fix N1QL injection vulnerability in build_update where nested Hash values (#{key}.#{k} = #{v}) were interpolated directly into the query string without sanitization
  • Use quote(v) to properly escape user-supplied values

Context

In Relation#build_update, when updating nested Hash attributes (e.g. update_all(address: { city: user_input })), the value was interpolated raw into the N1QL query string. This allowed N1QL injection through crafted values.

Test plan

  • Existing update_all specs pass (nested attribute updates with _for clauses)
  • /test --fail-fast=false

🤖 Generated with Claude Code

In build_update, Hash values were interpolated directly into the query
string without sanitization, allowing N1QL injection. Use quote() to
properly escape values.

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Couchbase ORM relation builder to properly quote hash values when constructing update queries. The review feedback highlights a potential N1QL syntax error when a value is nil, as the quoted value would interpolate to an empty string, and suggests adding a fallback to 'NULL'.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread lib/couchbase-orm/relation.rb Outdated
When a nil value is passed in a Hash update (e.g. update(field: { key: nil })),
quote() returns nil which interpolates as an empty string, producing invalid
N1QL syntax. Fall back to 'NULL' literal in that case.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
if value.is_a?(Hash)
value.map do |k, v|
"#{key}.#{k} = #{v}"
"#{key}.#{k} = #{@model.quote(v) || 'NULL'}"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

A la place du || NULL, une meilleure approche serait de corriger quote elle-même pour retourner 'NULL' au lieu de nil — ce qui fixerait le problème à la source pour tous les appelants. Mais dans le cadre de cette PR (qui ne touche que le cas Hash), le fix local proposé par Gemini est raisonnable.

Cover string values, nil values, and special character quoting
in Hash-style update_all to verify the N1QL injection fix.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

1 participant