diff --git a/AGENTS.md b/AGENTS.md index d9d23c57..fe5a72e5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -301,8 +301,9 @@ plugin membership) is authored here. Run before opening any PR. Steps 1–2 mirror the CI gates; step 3 is a best-effort local lint that CI does not currently enforce but that keeps workflow changes clean: -The JSON-field guard needs Go 1.22 or later when a plugin carries Go support source. It builds only -its installed syntax decoder, reads comments and decoded literal strings/argument blocks, and never +The JSON-field guard needs Go 1.22 or later for every scanned surface. It builds only +its installed observer, joins adjacent literal shell quotes without evaluation, requires unique +decoded JSON keys, reads Go comments and decoded literal strings/argument blocks, and never executes the inspected package. Unresolved groupings beside a known JSON flag, malformed source, incomplete decoding, or exhausted source/decoded-work budgets stay UNKNOWN. This is not analysis of arbitrary Go runtime behavior. diff --git a/scripts/gh-json-go/main.go b/scripts/gh-json-go/main.go index 74884500..5a2a62a9 100644 --- a/scripts/gh-json-go/main.go +++ b/scripts/gh-json-go/main.go @@ -179,8 +179,109 @@ func guidance(source []byte) ([]string, error) { return d.parts, nil } +// normalizeShellFields joins only adjacent literal fragments of advertised field words. +// Markdown delimiters end a word; expansions and unresolved quoting never establish a clean scan. +func normalizeShellFields(source string) (string, error) { + letter := func(c byte) bool { return c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c == ',' } + space := func(c byte) bool { return c == ' ' || c == '\t' || c == '\n' || c == '\r' } + var output strings.Builder + position := 0 + for position < len(source) { + relative := strings.Index(source[position:], "--json") + if relative < 0 { + output.WriteString(source[position:]) + break + } + flag := position + relative + start := flag + len("--json") + // A quoted flag is one argument, while a closing Markdown backtick is a boundary. + if start < len(source) && (source[start] == '\'' || source[start] == '"') && flag > 0 && source[flag-1] == source[start] { + start++ + } + if start == len(source) || !(space(source[start]) || source[start] == '=' || source[start] == ',') { + output.WriteString(source[position:start]) + position = start + continue + } + separatorStart := start + for start < len(source) && (space(source[start]) || source[start] == '=' || source[start] == ',') { + start++ + } + end := start + var word strings.Builder + for end < len(source) { + c := source[end] + if c == '$' { + return "", fmt.Errorf("JSON field word contains unresolved expansion") + } + if c == '\\' { + return "", fmt.Errorf("JSON field word contains an unresolved escape") + } + if letter(c) { + word.WriteByte(c) + end++ + continue + } + if c == '`' { + separator := source[separatorStart:end] + if word.Len() == 0 && strings.HasPrefix(source[end:], "```") && + strings.Contains(separator, "\n") && strings.Trim(separator, " \t\r\n") == "" { + break // A following Markdown fence is not part of a field word. + } + line := strings.LastIndexByte(source[:flag], '\n') + 1 + if strings.Count(source[line:flag], "`")%2 == 0 { + return "", fmt.Errorf("JSON field word contains unresolved command substitution") + } + break // Close the Markdown span that opened before this command. + } + if c != '\'' && c != '"' { + break + } + closing := strings.IndexByte(source[end+1:], c) + if closing < 0 { + line := strings.LastIndexByte(source[:flag], '\n') + 1 + if strings.Count(source[line:flag], string(c))%2 == 0 || + end+1 < len(source) && (letter(source[end+1]) || source[end+1] == '$') { + return "", fmt.Errorf("JSON field quoting is incomplete") + } + break // A surrounding prose quote can close after the final field. + } + closing += end + 1 + fragment := source[end+1 : closing] + literal := true + for i := 0; i < len(fragment); i++ { + literal = literal && letter(fragment[i]) + } + if !literal { + if strings.ContainsAny(fragment, "$`\\") { + return "", fmt.Errorf("JSON field quoting contains unresolved expansion") + } + break // Ordinary prose outside the literal field word remains a boundary. + } + word.WriteString(fragment) + end = closing + 1 + } + output.WriteString(source[position:start]) + output.WriteString(word.String()) + position = end + } + return output.String(), nil +} + // run reads a bounded retained snapshot and publishes only complete decoded guidance. func run() error { + if len(os.Args) == 2 && os.Args[1] == "--shell-fields" { + input, err := io.ReadAll(io.LimitReader(os.Stdin, (8<<20)+1)) + if err != nil || len(input) > 8<<20 { + return fmt.Errorf("guidance text exceeds the complete observation budget") + } + text, err := normalizeShellFields(string(input)) + if err != nil { + return err + } + _, err = io.WriteString(os.Stdout, text) + return err + } if len(os.Args) != 2 { return fmt.Errorf("one retained source path is required") } diff --git a/scripts/guard-gh-json-fields.sh b/scripts/guard-gh-json-fields.sh index e9334ee4..6c6f4f95 100755 --- a/scripts/guard-gh-json-fields.sh +++ b/scripts/guard-gh-json-fields.sh @@ -15,7 +15,9 @@ # Scans every *.md, *.txt, *.json, *.jq and *.go under ROOT/plugins; any other non-script file is UNKNOWN. Shell # scripts are not scanned: a script with a bad field fails loudly the first time it runs, whereas prose # silently misleads every agent that reads it. -# Go needs Go 1.22+: only the installed syntax decoder is built. It reads comments, decoded literal +# Go 1.22+ is required: only the installed observer is built. Literal adjacent shell quotes are +# joined without evaluating inspected text; expansions are UNKNOWN. JSON requires unique decoded +# keys. Normalized text is bounded to 8 MiB. The Go decoder reads comments, decoded literal # strings and literal Command/CommandContext or composite argument blocks. Unresolved groupings beside # a known JSON flag, malformed source, or the 8 MiB source / 4 MiB decoded-work / 262144-step budgets # are UNKNOWN. This does not evaluate arbitrary Go programs. @@ -30,6 +32,7 @@ # which would make a 0 meaningless). set -euo pipefail +export LC_ALL=C root="${1:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)}" helper_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) @@ -39,6 +42,19 @@ unknown() { exit 2 } +# shellcheck source=scripts/json-object.lib.sh +. "$helper_dir/json-object.lib.sh" || unknown 'cannot load the complete JSON observer' + +# Build only the installed observer; inspected packages are never compiled or executed. +ensure_decoder() { + if [[ ! -x $go_decoder ]]; then + command -v go >/dev/null || return 2 + GOENV=off GOWORK=off GO111MODULE=off GOTOOLCHAIN=local GOFLAGS='' CGO_ENABLED=0 \ + GOOS='' GOARCH='' GOCACHEPROG='' GOTMPDIR="$observation_dir" \ + go build -o "$go_decoder" "$helper_dir/gh-json-go/main.go" || return 2 + fi +} + # Emit the retained surface as plain text. $1 selects its type; $2 contains its observed bytes. # JSON is DECODED, never pattern-matched: a \u escape of a letter IS # that letter, so an escaped `merged` is still `merged`. Each decoded string is followed by `%`, which ends a field @@ -48,12 +64,7 @@ decode_surface() { # Parse retained source and decode comments/literals, including literal command argv. # Only this installed decoder is built, never the inspected Go package. *.go) - if [[ ! -x $go_decoder ]]; then - command -v go >/dev/null || return 2 - GOENV=off GOWORK=off GO111MODULE=off GOTOOLCHAIN=local GOFLAGS='' CGO_ENABLED=0 \ - GOOS='' GOARCH='' GOCACHEPROG='' GOTMPDIR="$observation_dir" \ - go build -o "$go_decoder" "$helper_dir/gh-json-go/main.go" || return 2 - fi + ensure_decoder || return 2 "$go_decoder" "$2" ;; # Object KEYS are scanned as well as values. An argv list (an all-string array under an `args`, # `argv`, `cmd` or `command` key, e.g. ["pr","view","--json","state,merged"]) is ALSO emitted @@ -78,9 +89,11 @@ decode_surface() { # in SHELL quotes ('--json') is unwrapped first — that is one argument to the shell — while a # backtick-wrapped one stays a Markdown code span. extract_lists() { + ensure_decoder || return 2 decode_surface "$1" "$2" \ | tr -d '\000' \ | awk '{ if (sub(/\\$/, "")) { printf "%s", $0 } else { print } }' \ + | "$go_decoder" --shell-fields \ | sed -E -e 's/\\[nrt]/ /g' -e 's/\\/ /g' \ | awk '{ line = $0; sub(/[[:space:]]+$/, "", line) @@ -202,8 +215,8 @@ for surface in "${surfaces[@]}"; do cat "${surface}" > "${snapshot}" || unknown "${surface#"${root}/"} could not be completely read" fi case "${surface}" in - *.json) jq empty "${snapshot}" >/dev/null 2>&1 || - unknown "${surface#"${root}/"} does not parse, so any field it prescribes would go unseen" ;; + *.json) json_value_unique "${snapshot}" || + unknown "${surface#"${root}/"} is incomplete or has repeated decoded keys" ;; esac scanned=$((scanned + 1)) # Retain one complete observation. A failed stage may already have emitted valid-looking diff --git a/scripts/guard-gh-json-fields.test.sh b/scripts/guard-gh-json-fields.test.sh index 53ce4dfe..2f26534a 100755 --- a/scripts/guard-gh-json-fields.test.sh +++ b/scripts/guard-gh-json-fields.test.sh @@ -73,6 +73,47 @@ done < <(printf '%s\0' \ 'boundary: see `gh pr view --json comments`, merged PRs need no polling' \ 'boundary: `gh pr view --json state`, and merged ones are done') +# Repeated decoded keys must be rejected before extraction can discard an earlier prescription. +for document in \ + '{"prompt":"gh pr view --json merged","prompt":"gh pr view --json mergedAt"}' \ + '{"prompt":"gh pr view --json merged","pro\u006dpt":"gh pr view --json mergedAt"}' \ + '{"nested":{"args":["--json","merged"],"args":["--json","mergedAt"]}}' \ + '[{"prompt":"gh pr view --json merged","prompt":"safe"}]'; do + dir="$(fixture "unknown-duplicate-json-$passed")" + printf '%s\n' "$document" > "$dir/plugins/p/case.json" + expect 2 'duplicate decoded JSON keys are UNKNOWN' "$dir" +done + +# Adjacent quotes belong to the same shell word; unresolved literal combinations remain UNKNOWN. +for fields in 'state,mer""ged' "state,mer''ged" '"state,mer""ged"' "state,'mer'ged"; do + dir="$(fixture "bad-adjacent-literals-$passed")" + printf 'gh pr view --json %s\n' "$fields" > "$dir/plugins/p/agents/case.md" + expect 1 'adjacent literal field fragments still prescribe merged' "$dir" +done +for fields in 'state,mer""gedAt' "state,mer'gedBy'" '"state,mer""geCommit"'; do + dir="$(fixture "good-adjacent-literals-$passed")" + printf 'gh pr view --json %s\n' "$fields" > "$dir/plugins/p/agents/case.md" + expect 0 'adjacent valid literal fields remain valid' "$dir" +done +dir="$(fixture unknown-adjacent-expansion)" +printf '%s\n' 'gh pr view --json state,mer"$fragment"ged' > "$dir/plugins/p/agents/case.md" +expect 2 'unresolved adjacent field expansion is UNKNOWN' "$dir" +for command in 'gh pr view --json state,mer${suffix}' 'gh pr view --json state,mer$(printf ged)' "gh pr view --json state,mer\$'ged'" "gh pr view --json \$'merged'"; do + dir="$(fixture "unknown-unquoted-expansion-$passed")" + printf '%s\n' "$command" > "$dir/plugins/p/agents/case.md" + expect 2 'unquoted or ANSI-C field expansion is UNKNOWN' "$dir" +done +for command in 'gh pr view --json state,mer\ged' 'gh pr view --json state,mer`printf ged`' 'gh pr view --json `printf merged`' 'gh pr view --json ```printf merged```'; do + dir="$(fixture "unknown-unquoted-shell-syntax-$passed")" + printf '%s\n' "$command" > "$dir/plugins/p/agents/case.md" + expect 2 'unquoted shell syntax inside a field word is UNKNOWN' "$dir" +done +for command in "gh pr view --json state,mer'" 'gh pr view --json state,mer"'; do + dir="$(fixture "unknown-incomplete-field-quote-$passed")" + printf '%s\n' "$command" > "$dir/plugins/p/agents/case.md" + expect 2 'an incomplete quote attached to a field word is UNKNOWN' "$dir" +done + # JSON surfaces are decoded: a \u-escaped letter is still that letter, so an escaped field name is caught. dir="$(fixture bad-json-escaped-name)" printf '{"prompt":"gh pr view --json state,\x5cu006derged,mergedAt"}\n' > "${dir}/plugins/p/plugin.json" @@ -127,6 +168,10 @@ dir="$(fixture good-closing-backtick)" printf '%s\n' 'The flag is `--json`' 'merged pull requests need no polling.' > "${dir}/plugins/p/agents/case.md" expect 0 "a closing backtick after --json, then 'merged' on the next line" "${dir}" +dir="$(fixture good-closing-fence-after-empty-list)" +printf '%s\n' '```bash' 'gh status --json' '```' > "${dir}/plugins/p/agents/case.md" +expect 0 'a closing Markdown fence after --json with no field list' "${dir}" + # A flag wrapped in shell quotes is still the flag: `gh pr view 42 '--json' state,merged`. dir="$(fixture bad-quoted-flag)" printf '%s\n' "gh pr view 42 '--json' state,merged" > "${dir}/plugins/p/agents/case.md"