v0.1.1.0 feat: optional remote URL in .secrets-store (EGB-282) (#2)
* chore: ignore .gstack/ (per-project local state)
* feat: optional remote URL in .secrets-store (EGB-282)
A second whitespace-separated token after the store name in .secrets-store
is treated as the store's git remote URL. When a teammate clones a project
bound to a store they don't have locally yet, the directed missing-store
error now fills in `git clone <url> <path>` so they can copy-paste instead
of asking the original setter for the URL.
Backward compatible: single-token .secrets-store files (the v0.1.0.x
format) continue to work and produce the existing `<their-store-remote>`
placeholder.
Security hardening (caught by adversarial review during /ship):
- The rendered git clone line is meant to be copy-pasted by a teammate.
Without sanitization, `work evil.git;rm -rf ~` would render verbatim
and execute `rm -rf ~` on paste. The parser now rejects URLs containing
shell metacharacters (;&|<>$`(){}*?!"'\\), control characters (incl.
ANSI escape sequences that could spoof terminal output), and embedded
whitespace. Rejected URLs are dropped with a stderr warning; the error
falls back to the safe placeholder.
- Switched from `set -- $line` to `read -r spec rest` so the URL field
isn't glob-expanded or word-split — important so `work *` from a
populated directory doesn't leak filenames into the URL field.
Tests 72 → 80. New: backward compat, SSH+HTTPS+~/-prefix URL forms,
comment-and-URL form, four named injection vectors (shell metachar,
backtick, $(), ANSI escape), multi-token URL, glob char, and a positive
test asserting standard git URL chars round-trip unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: bump version and changelog (v0.1.1.0)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Brian Majewski
GitHub
parent
1bb729f73b
commit
7c3a76e8c1
7 changed files with 296 additions and 20 deletions
|
|
@ -874,3 +874,174 @@ PKG
|
|||
[ "$status" -ne 0 ]
|
||||
[[ "$output" == *"HOME"* ]]
|
||||
}
|
||||
|
||||
# ─── EGB-282: optional remote URL in .secrets-store ──────────────────
|
||||
|
||||
@test "EGB-282: .secrets-store without URL still works (backward compat)" {
|
||||
unset SECRETS_DIR
|
||||
mkdir -p "$HOME/.secrets-work"
|
||||
mkdir -p "$WORK_DIR/proj"
|
||||
# Single token only — same as v0.1.0.0 format
|
||||
echo "work" > "$WORK_DIR/proj/.secrets-store"
|
||||
cd "$WORK_DIR/proj"
|
||||
run "$SECRETS_BIN" which
|
||||
[ "$status" -eq 0 ]
|
||||
[[ "$output" == *"$HOME/.secrets-work"* ]]
|
||||
}
|
||||
|
||||
@test "EGB-282: .secrets-store with URL parses both tokens" {
|
||||
unset SECRETS_DIR
|
||||
mkdir -p "$WORK_DIR/proj"
|
||||
echo "work git@github.com:acme/work-secrets.git" > "$WORK_DIR/proj/.secrets-store"
|
||||
cd "$WORK_DIR/proj"
|
||||
# Store doesn't exist yet — pull should fail with the directed error
|
||||
# that includes the actual URL (not the placeholder).
|
||||
run "$SECRETS_BIN" pull
|
||||
[ "$status" -eq 1 ]
|
||||
[[ "$output" == *"git clone git@github.com:acme/work-secrets.git $HOME/.secrets-work"* ]]
|
||||
# Placeholder must NOT appear when a real URL was supplied
|
||||
[[ "$output" != *"<their-store-remote>"* ]]
|
||||
}
|
||||
|
||||
@test "EGB-282: missing-store error still works without URL (placeholder)" {
|
||||
unset SECRETS_DIR
|
||||
mkdir -p "$WORK_DIR/proj"
|
||||
echo "missing-only" > "$WORK_DIR/proj/.secrets-store"
|
||||
cd "$WORK_DIR/proj"
|
||||
run "$SECRETS_BIN" pull
|
||||
[ "$status" -eq 1 ]
|
||||
# No URL given — placeholder is the right behavior.
|
||||
[[ "$output" == *"<their-store-remote>"* ]]
|
||||
}
|
||||
|
||||
@test "EGB-282: https URL is preserved literally" {
|
||||
unset SECRETS_DIR
|
||||
mkdir -p "$WORK_DIR/proj"
|
||||
echo "work https://github.com/acme/work-secrets.git" > "$WORK_DIR/proj/.secrets-store"
|
||||
cd "$WORK_DIR/proj"
|
||||
run "$SECRETS_BIN" pull
|
||||
[ "$status" -eq 1 ]
|
||||
[[ "$output" == *"https://github.com/acme/work-secrets.git"* ]]
|
||||
}
|
||||
|
||||
@test "EGB-282: ~/-prefixed path with URL works" {
|
||||
unset SECRETS_DIR
|
||||
mkdir -p "$WORK_DIR/proj"
|
||||
printf '~/.secrets-x git@github.com:acme/x.git\n' > "$WORK_DIR/proj/.secrets-store"
|
||||
cd "$WORK_DIR/proj"
|
||||
run "$SECRETS_BIN" pull
|
||||
[ "$status" -eq 1 ]
|
||||
[[ "$output" == *"git clone git@github.com:acme/x.git $HOME/.secrets-x"* ]]
|
||||
}
|
||||
|
||||
@test "EGB-282: comments before URL line are still skipped" {
|
||||
unset SECRETS_DIR
|
||||
mkdir -p "$WORK_DIR/proj"
|
||||
printf '# this binding was set by alice\n# please do not delete\nwork git@github.com:acme/work-secrets.git\n' > "$WORK_DIR/proj/.secrets-store"
|
||||
cd "$WORK_DIR/proj"
|
||||
run "$SECRETS_BIN" pull
|
||||
[ "$status" -eq 1 ]
|
||||
[[ "$output" == *"git clone git@github.com:acme/work-secrets.git"* ]]
|
||||
}
|
||||
|
||||
# ─── EGB-282 adversarial regressions: URL injection prevention ────────
|
||||
|
||||
@test "EGB-282 SECURITY: URL with shell metachars is dropped (rm -rf payload)" {
|
||||
unset SECRETS_DIR
|
||||
mkdir -p "$WORK_DIR/proj"
|
||||
# The classic copy-paste shell injection: a `;` after the "URL" splits
|
||||
# the rendered git clone into two commands, the second of which is the
|
||||
# attacker payload. The teammate copy-pasting the directed-error one-liner
|
||||
# would execute `rm -rf ~`. The parser must reject this.
|
||||
printf 'work evil.git;rm -rf ~\n' > "$WORK_DIR/proj/.secrets-store"
|
||||
cd "$WORK_DIR/proj"
|
||||
run "$SECRETS_BIN" pull
|
||||
[ "$status" -eq 1 ]
|
||||
# Must use the placeholder, NOT the attacker URL
|
||||
[[ "$output" == *"<their-store-remote>"* ]]
|
||||
[[ "$output" != *"rm -rf"* ]]
|
||||
# And must have warned the user that something was dropped
|
||||
[[ "$output" == *"WARNING"* ]]
|
||||
[[ "$output" == *"unsafe"* ]]
|
||||
}
|
||||
|
||||
@test "EGB-282 SECURITY: URL with backticks is dropped" {
|
||||
unset SECRETS_DIR
|
||||
mkdir -p "$WORK_DIR/proj"
|
||||
printf 'work evil.git`whoami`\n' > "$WORK_DIR/proj/.secrets-store"
|
||||
cd "$WORK_DIR/proj"
|
||||
run "$SECRETS_BIN" pull
|
||||
[ "$status" -eq 1 ]
|
||||
[[ "$output" == *"<their-store-remote>"* ]]
|
||||
}
|
||||
|
||||
@test "EGB-282 SECURITY: URL with command substitution \$() is dropped" {
|
||||
unset SECRETS_DIR
|
||||
mkdir -p "$WORK_DIR/proj"
|
||||
printf 'work evil.git$(whoami)\n' > "$WORK_DIR/proj/.secrets-store"
|
||||
cd "$WORK_DIR/proj"
|
||||
run "$SECRETS_BIN" pull
|
||||
[ "$status" -eq 1 ]
|
||||
[[ "$output" == *"<their-store-remote>"* ]]
|
||||
}
|
||||
|
||||
@test "EGB-282 SECURITY: URL with ANSI escape is dropped (terminal-spoof prevention)" {
|
||||
unset SECRETS_DIR
|
||||
mkdir -p "$WORK_DIR/proj"
|
||||
# ESC ([ \x1b) lets a malicious URL render differently from what gets pasted
|
||||
printf 'work evil.git\x1b[2K\r\n' > "$WORK_DIR/proj/.secrets-store"
|
||||
cd "$WORK_DIR/proj"
|
||||
run "$SECRETS_BIN" pull
|
||||
[ "$status" -eq 1 ]
|
||||
[[ "$output" == *"<their-store-remote>"* ]]
|
||||
}
|
||||
|
||||
@test "EGB-282 SECURITY: multi-token URL ('work url1 url2') is dropped" {
|
||||
unset SECRETS_DIR
|
||||
mkdir -p "$WORK_DIR/proj"
|
||||
# `git clone url1 url2 /path` would clone url1 into directory `url2` — wrong
|
||||
# behavior either way. Treat any whitespace inside the URL token as unsafe.
|
||||
printf 'work url1 url2\n' > "$WORK_DIR/proj/.secrets-store"
|
||||
cd "$WORK_DIR/proj"
|
||||
run "$SECRETS_BIN" pull
|
||||
[ "$status" -eq 1 ]
|
||||
[[ "$output" == *"<their-store-remote>"* ]]
|
||||
}
|
||||
|
||||
@test "EGB-282 SECURITY: glob char in URL is dropped (no expansion either way)" {
|
||||
unset SECRETS_DIR
|
||||
mkdir -p "$WORK_DIR/proj"
|
||||
# In v0.1.1.0-alpha (set -- $line), this used to expand to filenames.
|
||||
# The fixed parser uses `read -r`, so it doesn't glob — but glob chars
|
||||
# are still rejected as suspicious.
|
||||
printf 'work /tmp/*\n' > "$WORK_DIR/proj/.secrets-store"
|
||||
cd "$WORK_DIR/proj"
|
||||
run "$SECRETS_BIN" pull
|
||||
[ "$status" -eq 1 ]
|
||||
[[ "$output" == *"<their-store-remote>"* ]]
|
||||
}
|
||||
|
||||
@test "EGB-282: spec parsing is glob-safe (work * does NOT expand)" {
|
||||
unset SECRETS_DIR
|
||||
mkdir -p "$WORK_DIR/proj"
|
||||
# Create files in cwd that would expand if `set -- $line` were used.
|
||||
touch "$WORK_DIR/proj/file1" "$WORK_DIR/proj/file2"
|
||||
printf 'work *\n' > "$WORK_DIR/proj/.secrets-store"
|
||||
cd "$WORK_DIR/proj"
|
||||
run "$SECRETS_BIN" which
|
||||
[ "$status" -eq 0 ]
|
||||
# Spec is the literal "work" (resolves to ~/.secrets-work). The "*" gets
|
||||
# rejected as unsafe URL and dropped. Resolution works; no globbing.
|
||||
[[ "$output" == *"$HOME/.secrets-work"* ]]
|
||||
}
|
||||
|
||||
@test "EGB-282: URL with - + _ : / @ . is preserved (positive test)" {
|
||||
unset SECRETS_DIR
|
||||
mkdir -p "$WORK_DIR/proj"
|
||||
# Standard git URL chars must NOT be rejected
|
||||
printf 'work git+ssh://user@host:2222/path/to-repo_v2.git\n' > "$WORK_DIR/proj/.secrets-store"
|
||||
cd "$WORK_DIR/proj"
|
||||
run "$SECRETS_BIN" pull
|
||||
[ "$status" -eq 1 ]
|
||||
[[ "$output" == *"git+ssh://user@host:2222/path/to-repo_v2.git"* ]]
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue