fix: validate team names to prevent teams/ path traversal (#140)#147
Merged
Conversation
Team names were used directly as filesystem path segments in the team registry (teams/<name>/config.json). A name containing "/", "\", or equal to "." / ".." could escape teams/ and create, read, move, or delete config files and team directories outside the agmsg state tree — via join.sh, leave.sh, team.sh, rename.sh, and rename-team.sh. Add scripts/lib/validate.sh with agmsg_validate_team_name and call it at every entry point that turns a team name into a path. It is a deny-list of path-dangerous constructs (empty, ".", "..", "/", "\", leading "-", control characters), NOT an ASCII allow-list — arbitrary UTF-8 team names (e.g. Japanese names already in use) stay valid; multibyte UTF-8 bytes are all >= 0x80 and never match the control-character class. scripts/lib/ ships via the installer's recursive copy, so no install change is needed. Tests: traversal / "."/".." / leading-dash / empty rejection across join/leave/team/rename/rename-team, no-escape assertions, and a UTF-8 name acceptance regression (test_team.bats). Closes #140
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.
What
Fixes #140 — team names were used directly as filesystem path segments in the team registry (
teams/<name>/config.json), so a name containing/,\, or equal to./..could escapeteams/and create/read/move/delete config files and team directories outside the agmsg state tree.Affected entry points:
join.sh,leave.sh,team.sh,rename.sh,rename-team.sh.How
scripts/lib/validate.sh→agmsg_validate_team_name, called at every entry point that turns a team name into a path..,..,/,\, leading-, control characters), not an ASCII allow-list. Team names are intentionally arbitrary UTF-8 (Japanese team names are already in use); multibyte UTF-8 bytes are all>= 0x80and never match the control-character class, so legitimate names stay valid.scripts/lib/ships via the installer's recursive copy — no install change needed.Verification
The issue's repro is now rejected (exit 1, no escaped files), and legitimate ASCII + UTF-8 names still join:
Tests added to
tests/test_team.bats: traversal /.+../ leading-dash / empty rejection across all five scripts, no-escape assertions for join and rename-team, and a UTF-8 acceptance regression.bats tests/test_team.bats→ 42/42.Closes #140