fix: escape interpolated values as SQL string literals#223
Conversation
send.sh, rename.sh and rename-team.sh interpolate team/from/to (and old/new names) directly into SQL string literals, escaping only body. A team or agent name containing a single quote breaks the INSERT/UPDATE (correctness) and is an injection surface. Escape every interpolated value with the same sed idiom already used for body, via a small _agmsg_sqlesc helper.
, #87) rename.sh and rename-team.sh interpolated team/agent names directly into their messages/events UPDATE string literals, escaping nothing. A name with a single quote (validate.sh only blocks path traversal) broke the UPDATE and was an injection surface that could widen the WHERE predicate. - add a driver-agnostic agmsg_sqlesc helper (same semantics as the sqlite driver's _sqlite_lit / storage_send escaping) - escape every interpolated value in the rename / rename-team UPDATEs - rename-team's config read switches from `.param set` (whose dot-command tokenizer does not honour SQL '' escaping, so apostrophe content breaks the binding) to the readfile() idiom join.sh already uses, so a quoted team name round-trips end to end - regression tests: a quoted team migrates only its own messages (no predicate widening); the rename messages UPDATE stays scoped rename.sh still interpolates agent names into json *paths* ($.agents.<name>) for the config lookup, so quoted-agent rename stays gated there pending the separate json-path handling decision (#87 follow-up); the UPDATE escaping is defense-in-depth that also protects legacy rows.
|
Thanks for catching this — interpolating team/agent names straight into SQL string literals is a real correctness + injection issue, and your report made the gap concrete. We've folded equivalent escaping into the storage-axis work (#221) rather than merging this directly, since that PR reworks the same call sites: send is already parameterized through the storage facade there, and we extended the same doubled-quote escaping to the rename / rename-team So this is addressed, with credit to you for the diagnosis. We'll close this in favor of #221 once that lands (it's held for the 1.1.2 release); leaving it open until then so the trail is clear. One narrower piece — escaping/validating names interpolated into JSON paths (e.g. the rename config lookup) — is a separate design question we're tracking as a follow-up. Thanks again. |
2 participants
send.sh, rename.sh and rename-team.sh interpolate team/from/to (and old/new names) directly into SQL string literals, escaping only body. A team or agent name containing a single quote breaks the INSERT/UPDATE (correctness) and is an injection surface. Escape every interpolated value with the same sed idiom already used for body, via a small _agmsg_sqlesc helper.