Refactor kv commands: replace inline params in SQL queries (#15108)

# Description

Update some comments and fix potential security issue:

SQL Injection in DELETE statements: The code constructs SQL queries by
interpolating the $key variable directly into the string. If a key
contains malicious input could lead to SQL injection. Need to use
parameterized queries or escaping.
This commit is contained in:
Justin Ma 2025-02-14 12:23:59 +08:00 committed by GitHub
parent e1c5ae3cd5
commit 453e294883
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -14,8 +14,7 @@
# Stores the pipeline value for later use # Stores the pipeline value for later use
# #
# If the key already exists, it is updated # If the key already exists, it is updated to the new value provided.
# to the new value provided.
@example "Store the list of files in the home directory" { @example "Store the list of files in the home directory" {
ls ~ | kv set "home snapshot" ls ~ | kv set "home snapshot"
} }
@ -29,7 +28,7 @@ export def "kv set" [
key: string key: string
value_or_closure?: any value_or_closure?: any
--return (-r): string # Whether and what to return to the pipeline output --return (-r): string # Whether and what to return to the pipeline output
--universal (-u) --universal (-u) # Store the key-value pair in a universal database
] { ] {
# Pipeline input is preferred, but prioritize # Pipeline input is preferred, but prioritize
# parameter if present. This allows $in to be # parameter if present. This allows $in to be
@ -53,7 +52,7 @@ export def "kv set" [
let db_open = (db_setup --universal=$universal) let db_open = (db_setup --universal=$universal)
try { try {
# Delete the existing key if it does exist # Delete the existing key if it does exist
do $db_open | query db $"DELETE FROM std_kv_store WHERE key = '($key)'" do $db_open | query db "DELETE FROM std_kv_store WHERE key = :key" --params { key: $key }
} }
match $universal { match $universal {
@ -97,36 +96,34 @@ export def "kv set" [
# Retrieves a stored value by key # Retrieves a stored value by key
# #
# Counterpart of "kv set". Returns null # Counterpart of "kv set". Returns null if the key is not found.
# if the key is not found.
@example "Retrieve a stored value" { @example "Retrieve a stored value" {
kv get foo kv get foo
} }
export def "kv get" [ export def "kv get" [
key: string # Key of the kv-pair to retrieve key: string # Key of the kv-pair to retrieve
--universal (-u) --universal (-u) # Whether to use the universal db
] { ] {
let db_open = (db_setup --universal=$universal) let db_open = (db_setup --universal=$universal)
do $db_open do $db_open
# Hack to turn a SQLiteDatabase into a table # Hack to turn a SQLiteDatabase into a table
| $in.std_kv_store | wrap temp | get temp | $in.std_kv_store | wrap temp | get temp
| where key == $key | where key == $key
# Should only be one occurrence of each key in the stor # Should only be one occurrence of each key in the stor
| get -i value.0 | get -i value.0
| match $in { | match $in {
# Key not found # Key not found
null => null null => null
# Key found # Key found
_ => { from nuon } _ => { from nuon }
} }
} }
# List the currently stored key-value pairs # List the currently stored key-value pairs
# #
# Returns results as the Nushell value rather # Returns results as the Nushell value rather than the stored nuon.
# than the stored nuon.
export def "kv list" [ export def "kv list" [
--universal (-u) --universal (-u) # Whether to use the universal db
] { ] {
let db_open = (db_setup --universal=$universal) let db_open = (db_setup --universal=$universal)
@ -140,8 +137,8 @@ export def "kv list" [
# Returns and removes a key-value pair # Returns and removes a key-value pair
export def --env "kv drop" [ export def --env "kv drop" [
key: string # Key of the kv-pair to drop key: string # Key of the kv-pair to drop
--universal (-u) --universal (-u) # Whether to use the universal db
] { ] {
let db_open = (db_setup --universal=$universal) let db_open = (db_setup --universal=$universal)
@ -150,7 +147,7 @@ export def --env "kv drop" [
try { try {
do $db_open do $db_open
# Hack to turn a SQLiteDatabase into a table # Hack to turn a SQLiteDatabase into a table
| query db $"DELETE FROM std_kv_store WHERE key = '($key)'" | query db "DELETE FROM std_kv_store WHERE key = :key" --params { key: $key }
} }
if $universal and ($env.NU_KV_UNIVERSALS? | default false) { if $universal and ($env.NU_KV_UNIVERSALS? | default false) {
@ -163,12 +160,12 @@ export def --env "kv drop" [
def universal_db_path [] { def universal_db_path [] {
$env.NU_UNIVERSAL_KV_PATH? $env.NU_UNIVERSAL_KV_PATH?
| default ( | default (
$nu.data-dir | path join "std_kv_variables.sqlite3" $nu.data-dir | path join "std_kv_variables.sqlite3"
) )
} }
def db_setup [ def db_setup [
--universal --universal # Whether to use the universal db
] : nothing -> closure { ] : nothing -> closure {
try { try {
match $universal { match $universal {
@ -181,7 +178,7 @@ def db_setup [
value: '' value: ''
} }
$dummy_record | into sqlite (universal_db_path) -t std_kv_store $dummy_record | into sqlite (universal_db_path) -t std_kv_store
open (universal_db_path) | query db $"DELETE FROM std_kv_store WHERE key = '($uuid)'" open (universal_db_path) | query db "DELETE FROM std_kv_store WHERE key = :key" --params { key: $uuid }
} }
false => { false => {
# Create the stor table if it doesn't exist # Create the stor table if it doesn't exist