Skip to content
This repository was archived by the owner on Oct 31, 2025. It is now read-only.

Conversation

@hermanschaaf
Copy link
Member

Potential fix for https://github.com/cloudquery/databricks-sql-go/security/code-scanning/1

General Approach:
To robustly construct valid JSON objects from untrusted keys and values, avoid building string representations by hand. Instead, use the encoding/json package to do the actual encoding (which always takes care of correct escaping and quoting), or — if hand-building is absolutely necessary due to performance or low-level requirements — make sure to JSON-escape and quote the key using standard library APIs such as json.Marshal.

Detailed Fix:
On line 301, instead of manually enclosing the key in double quotes (and trying to check if it's already quoted), we should always encode the key using json.Marshal(k) (not marshal(k), which encodes with custom logic) so that we get double-quoted, escaped key strings. This guarantees that, no matter what data is in k, the result will be valid JSON string key syntax.

  • Replace the logic starting at line 300–303: Remove the custom prefix check and always encode keys with json.Marshal.
  • For simplicity, within the existing context (which seems to gather up a string representation with own formatting), let’s use json.Marshal(k) to produce the key and insert it as-is.
  • If marshal(k) is needed because k could be other types (like numbers or objects), and this must match a previous design, be extra careful about not stripping or adding redundant quotes. But, for JSON object keys, only strings (or types convertible to strings) are valid, so coercion via json.Marshal is safest.
  • Update relevant error handling accordingly.

Affected lines:

  • In file internal/rows/arrowbased/columnValues.go, replace lines 300–303 with logic that always uses json.Marshal(k) for the object key and inserts the result directly, followed by a colon.
  • This may require updating the usage of key in context, possibly moving k from before, or using key, but marshaling it again as needed.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

hermanschaaf and others added 2 commits October 27, 2025 16:11
…quoting

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants