fix: close #87-class SQLi/path-hazard gaps in rename/leave/reset/rename-team/team/api - #482
Merged
Conversation
…team/team/api join.sh (PR272/#87) validated and SQL-escaped agent/team names but never spliced them into a raw JSON path; its sibling scripts did neither: - rename.sh, leave.sh, reset.sh spliced OLD_NAME/NEW_NAME/AGENT_ID/ TARGET_AGENT directly into '$.agents.<name>' path literals with no validation and no escaping — a name with a single quote could break out of the surrounding SQL statement, and one with '.', '/', '[', ']', or '"' could misroute the path to the wrong key. - reset.sh also spliced AGENT_TYPE unescaped into a SQL literal. - rename-team.sh set the renamed team's $.name field from an unescaped NEW_TEAM value. - api.sh's 'get teams <team> members' never validated <team> before splicing it into a filesystem path (path traversal). Fixed by wiring in the existing agmsg_validate_agent_name/ agmsg_validate_team_name gate at every entry point, and switching every $.agents.<name> lookup to concatenate an escaped SQL string literal ('$.agents.' || '<escaped>') rather than splicing the raw name into the path text. That surfaced a deeper, pre-existing bug the fix would otherwise have silently inherited: rename.sh/leave.sh/reset.sh/rename-team.sh/team.sh all read a team's config via '.param set :json '<escaped>'' — but the sqlite3 shell's dot-command tokenizer does not honour SQL '' escaping (confirmed directly: '.param set :x '\'a''b\''' errors instead of binding a''b), so this call silently mis-parsed for any config that already contained a single quote (e.g. one existing agent already named with a quote), corrupting every query built on top of it. team.sh's case was worst: it printed '.param'\''s own usage text as fake member rows with exit 0. Fixed every such call site to splice the (already-escaped) JSON blob as a genuine SQL string literal instead of binding it via '.param set'.
…set/rename-team/team/api
fujibee
added a commit
that referenced
this pull request
Jul 24, 2026
…r bug) The sqlite3 shell's dot-command tokenizer does not honour SQL '' escaping, so .param set :json '<escaped>' silently mis-parses as soon as a team's config already contains a single quote anywhere (confirmed directly: .param set :x 'a''b' errors instead of binding a'b). Splice the escaped JSON blob as a genuine SQL string literal in the actual SELECT statement instead, which does honour '' escaping — same fix as PR #482 applied to the sibling registry scripts.
fujibee
added a commit
that referenced
this pull request
Jul 24, 2026
…ndum)
koit-approved OSS interface addition for cloud/self-hosted crash recovery:
a driver's child 'connect' invocation may die between a successful
server-side exchange and the local commit finishing, leaving it unable to
tell whether that operation actually landed.
- 'remote status --json' emits a strict, secret-free object per team
({local_team, endpoint, server_instance_id, remote_team_id,
credential_id, state: active|disconnected}) a driver correlates against
its own operation-status record.
- 'remote pending list [--json]' / 'remote pending abort <pending_id>'
enumerate and clean up an orphaned exchange that never reached a local
commit. pending_id is the existing sha256(endpoint, token) digest
already used as the pending record's filename — content-derived and
opaque, so no separate generation counter is needed for ABA protection
(reusing an id requires the identical (endpoint, token) retry). list
always reports pending_id/endpoint even for a record whose content
fails strict validation (a legacy/corrupt record), so crash recovery can
still see and abort it; the raw credential is never read into the
listing path at all.
Building this surfaced the same #87-class '.param set' tokenizer bug
already fixed in key.sh (this commit) and PR #482 (registry scripts):
_remote_read_config_field, _remote_local_disconnect, and _remote_commit
all bound the config JSON the same broken way. Fixed identically —
splice the escaped blob as a genuine SQL string literal instead of
binding it via .param set.
Tests: 17 new cases in test_remote.bats covering both JSON schemas,
never-connected/disconnected states, corrupt/legacy pending records, and
a quote-containing team name exercising the .param set fix end to end.
Full test_key.bats + test_remote.bats suite green (73/73).
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
join.sh's #87/PR272 fix validated and SQL-escaped agent/team names before using them, but never spliced them into a raw JSON path string. Its sibling scripts did neither, leaving the same bug class open:
rename.sh,leave.sh,reset.shsplicedOLD_NAME/NEW_NAME/AGENT_ID/TARGET_AGENTdirectly into'$.agents.<name>'JSON path literals with no validation and no escaping — a name containing a single quote could break out of the surrounding SQL statement, and one containing.,/,[,], or"could misroute the path to the wrong key.reset.shalso splicedAGENT_TYPEunescaped into a SQL literal.rename-team.shset the renamed team's$.namefield from an unescapedNEW_TEAMvalue.api.sh'sget teams <team> membersnever validated<team>before splicing it into a filesystem path (path traversal).Fixed by wiring in the existing
agmsg_validate_agent_name/agmsg_validate_team_namegates at every entry point (mirroring join.sh), and switching every$.agents.<name>lookup to concatenate an escaped SQL string literal ('$.agents.' || '<escaped>') instead of splicing the raw name into the path text.A deeper bug this surfaced
Fixing the above the "obvious" way would have silently inherited a second, pre-existing bug:
rename.sh/leave.sh/reset.sh/rename-team.sh/team.shall read a team's config via.param set :json '<escaped>'— but the sqlite3 shell's dot-command tokenizer does not honour SQL''escaping (confirmed directly:.param set :x 'a''b'errors out instead of bindinga'b), so this call silently mis-parsed for any config that already contained a single quote (e.g. one pre-existing agent already named with a quote), corrupting every query built on top of it.team.sh's case was worst: it printed.param's own usage help text as fake member rows, with exit code 0 (false success). Fixed every such call site to splice the (already-escaped) JSON blob as a genuine SQL string literal instead of binding it through.param set— the same patternresolve-project.sh'sresolve_teamalready uses for the identical reason.Test plan
test_team.bats— added quote-SQLi and path-hazard-rejection tests forrename.sh,leave.sh,reset.sh,rename-team.sh,team.sh; full suite green (96/96)test_api.bats— added a path-traversal rejection test forget teams <team> members; full suite greentest_actas_integration.bats— unaffected, still green