diff --git a/skills/github-release/references/recovery-procedures.md b/skills/github-release/references/recovery-procedures.md index 103f1e5..14888af 100644 --- a/skills/github-release/references/recovery-procedures.md +++ b/skills/github-release/references/recovery-procedures.md @@ -325,6 +325,48 @@ See `ter-republish.md` for the TYPO3-specific pattern using a release notes, re-run only downstream publishers, never the full release workflow." +## Release Titles Differ From the Tag + +**Symptom**: Releases carry a title such as `QuickRoute v1.21.0` while the +convention here is the bare tag (`gh release create … --title "vX.Y.Z"`), +and the maintainer wants the existing ones renamed. + +**Cause**: The release workflow sets its own `--title`. The pattern +`--title " $TAG"` tends to arrive with a hand-written packaging +step and is copied from release to release without a reason. + +**Prevention**: Set `--title "$TAG"` in the workflow. A store upload that +lists files outside the repository (a CurseForge `displayName`, for +example) may keep the project name; the GitHub release page shows the +repository name already. + +**Recovery** (existing releases): this is the maintainer's step, not the +agent's. `guard-gh-release.py` blocks `gh release edit --title` and a +mutating `gh api` call on a releases endpoint, with no override, and that is +intended. It reads the command it is shown; a call wrapped in `bash -c "…"` +or in a script is beyond it, so do not read its silence there as permission. +Hand the maintainer a script instead, and let them run it with the `!` prefix: + +1. List the releases whose title is exactly ` `: + `gh api "repos/$R/releases?per_page=100" --paginate --jq '.[] | select(.name == " " + .tag_name) | "\(.id)\t\(.tag_name)"'`. +2. Dry run by default: print each planned rename and change nothing. + Rename only on `--apply`, and only titles that match exactly. +3. Per release: `gh api -X PATCH "repos/$R/releases/$ID" -f name="$TAG"`, + then read `.name` back and count a mismatch as a failure. Only the title + changes; tag, notes and assets stay. +4. Run the dry run yourself first and show its output with the `--apply` + command. The guard sees only the script call, not the `gh` calls inside + it, so it would let `--apply` through as well: leaving `--apply` to the + maintainer is your part, not something the guard enforces. + +Measured on CybotTM/wow-quickroute (2026-09-23): 20 of 30 releases carried +the prefixed title, none marked `immutable`; all 20 were renamed and read +back without a failure. Afterwards check that no release has a title +different from its tag: +`gh api "repos/$R/releases?per_page=100" --paginate --jq '.[] | select(.name != .tag_name) | .tag_name'` +must print nothing. A release with an empty `name` shows up here too; +GitHub displays its tag as the title, so it needs no rename. + ## Mis-Tagged SemVer Release (Scope Larger Than Version Bump Implies) **Symptom**: A release was tagged (and published, and consumed by TER / diff --git a/skills/github-release/scripts/_invocations.py b/skills/github-release/scripts/_invocations.py index c5e892e..a939013 100644 --- a/skills/github-release/scripts/_invocations.py +++ b/skills/github-release/scripts/_invocations.py @@ -17,6 +17,12 @@ # splitting it: a commit message holding a ";" is one invocation, not two. The # separator set carries the grouping constructs, so an invocation inside a # subshell, a brace group or a command substitution is still seen (issue #112). +# A brace separates only as a word of its own, as the shell's reserved word +# does: "{" or "}" with a blank or another separator on both sides. Inside a +# word it is text -- "repos/{owner}/{repo}/releases", "${R}", "{a,b}" -- and +# splitting there cut a gh api path in pieces the checks never saw. "${" with +# a blank after it opens bash 5.3's ${ cmd; } command substitution, so it +# separates as well. # # The escapes are not decoration. A double-quoted argument may contain \", and # reading that as the closing quote shifts every quote after it by one, which @@ -24,7 +30,7 @@ # separator outside quotes is likewise literal text, not a separator: the shell # passes it to the command rather than ending it. QUOTED_SPAN_OR_SEPARATOR = re.compile( - r"""\"(?:\\.|[^"\\])*\"|'[^']*'|\\.|(?P[;&|\n(){}]+)""" + r"""\"(?:\\.|[^"\\])*\"|'[^']*'|\\.|(?P[;&|\n()]+|\$\{(?=\s)|(? None: ALLOWED_RELEASE_SUBCOMMANDS = {"view", "list", "download"} # gh api calls to release endpoints with mutating methods. -# Matches patterns like: -# gh api repos/owner/repo/releases -X POST -# gh api /repos/owner/repo/releases --method DELETE -# gh api repos/owner/repo/releases/123 -X PATCH -GH_API_RELEASE_RE = re.compile( - INVOCATION_PREFIX - + r""" - gh\s+api\s+ # gh api - (?:(?:-\w+|--\w[\w-]*)(?:\s+(?:"[^"]*"|'[^']*'|\S+))?\s+)* # optional flags (e.g. -X POST, -H "...") - /?repos/[^\s]+/releases # release endpoint path - """, - re.VERBOSE, +# +# The call is read as the argv the shell will hand to gh, not matched as text. +# Two regexes did this before, and each shape they did not foresee was a way +# through: a quoted endpoint path ("repos/$R/releases/$ID"), a flag value with +# a space in it (-f body="new notes"), the long form --raw-field, --field=x. +# Their flags group also backtracked exponentially on a run of dash-words, +# which a 2-second hook timeout turns into a question of what the harness does +# on timeout. shlex splits the words the way the shell does, in linear time. +GH_API_RE = re.compile(INVOCATION_PREFIX + r"gh\s+api(?=\s|$)") + +# A release endpoint anywhere in a word: a bare or leading-slash path, a full +# URL, or a path whose quoting shlex could not resolve ($'...'). Owner and +# repository may sit in one segment, because a variable often holds both: +# "repos/$R/releases/$ID", "repos/$GITHUB_REPOSITORY/releases". A repository +# literally named "releases" is therefore read as a release path too, which +# errs toward blocking. +# The numeric route "repositories//releases" reaches the same releases. +_RELEASE_PATH_RE = re.compile( + r"(?:repos/(?:[^/\s]+/){1,2}|repositories/[^/\s]+/)releases(?:[/?#]|$)" ) -MUTATING_METHOD_RE = re.compile( - r""" - (?:-X|--method)\s*(POST|PUT|PATCH|DELETE) - """, - re.VERBOSE | re.IGNORECASE, +# A shell comment: an unquoted "#" that begins a word. Everything after it is +# text the shell never passes on. A "#" inside a word ("a#b", "releases/1#x") +# is part of that word. +_COMMENT_START = re.compile( + r"""\"(?:\\.|[^"\\])*\"|'[^']*'|\\.|(?P(?:^|(?<=\s))#)""" ) +def _without_comment(args: str) -> str: + """Cut a command's arguments at a real shell comment.""" + for match in _COMMENT_START.finditer(args): + if match.group("comment") is not None: + return args[: match.start()] + return args + + +# gh api flags that take a value, from `gh api --help` (gh 2.101.0). The value +# is consumed so it cannot be read as the endpoint. +_VALUE_FLAGS = { + "-X": "method", + "--method": "method", + "-f": "data", + "--raw-field": "data", + "-F": "data", + "--field": "data", + "--input": "data", + "-H": None, + "--header": None, + "-q": None, + "--jq": None, + "-t": None, + "--template": None, + "--hostname": None, + "--cache": None, + "-p": None, + "--preview": None, +} +_MUTATING_METHODS = {"POST", "PUT", "PATCH", "DELETE"} +_SAFE_METHODS = {"GET", "HEAD"} + + +def _gh_api_mutates_release(args: str): + """Return the method name if a gh api call mutates a release, else None. + + `args` is the text after `gh api`. The call is judged twice, with and + without a trailing shell comment cut off, and it mutates if either reading + says so. Cutting is needed: kept, a comment's words override the real + method ("-X DELETE # -X GET"). Cutting alone is not safe: the cutter is a + text scan, and a "#" bash does not treat as a comment -- inside ${V/ #/}, + or after a $'...' string with an escaped quote -- made it drop a real + "-X DELETE". Judging both ways, a mistake in either reading can only block. + A method or path that appears only in a comment therefore blocks too, which + is the safe direction. So does a release read whose trailing comment holds + an apostrophe ("# don't mutate"): the uncut reading sees an unclosed quote + and cannot parse it. Narrowing that was measured to reopen a 2 -> 0 path, + so the block stays; drop the apostrophe or the comment. + """ + return _judge_gh_api(_without_comment(args)) or _judge_gh_api(args) + + +def _judge_gh_api(args: str): + """One reading of a gh api call; see _gh_api_mutates_release. + + gh sends POST when a field or an input body is present and no method is + given, so data alone is a mutation unless the method is GET or HEAD (then + the fields become query parameters). + """ + try: + # No comments=True: shlex would treat a "#" inside a word as a + # comment, which bash does not ("-H X-A:a#b -X DELETE"). + words = shlex.split(args) + except ValueError: + # Unbalanced quotes: the shell will not run this as written, but a + # release path in it is reason enough not to guess. + return "UNPARSEABLE" if _RELEASE_PATH_RE.search(args) else None + method = None + has_data = False + positionals = [] + i = 0 + while i < len(words): + word = words[i] + name, value = None, None + if word.startswith("--"): + name, _, attached = word.partition("=") + value = attached if "=" in word else None + elif word.startswith("-") and len(word) > 1: + # A shorthand group, read as pflag reads it: boolean letters + # ("-i") are skipped, and the first letter that takes a value ends + # the group -- the rest of the word is its value ("-iXDELETE", + # "-if tag_name=v1"). + for j in range(1, len(word)): + if "-" + word[j] in _VALUE_FLAGS: + name = "-" + word[j] + rest = word[j + 1 :] + value = rest.removeprefix("=") or None + break + else: + positionals.append(word) + if name in _VALUE_FLAGS: + if value is None: + i += 1 + value = words[i] if i < len(words) else "" + kind = _VALUE_FLAGS[name] + if kind == "method": + method = value + elif kind == "data": + has_data = True + i += 1 + if not any(_RELEASE_PATH_RE.search(w) for w in positionals): + return None + if method is not None: + upper = method.upper() + if upper in _SAFE_METHODS: + return None + # A mutating method, or one the guard cannot read ("$M"). + return upper if upper in _MUTATING_METHODS else (method or "UNREADABLE") + return "POST" if has_data else None + + # Flags for gh release edit that modify metadata other than notes. # See: gh release edit --help # Uses \b word boundaries to avoid prefix collisions with future flags. @@ -277,18 +396,12 @@ def _check_invocation(cmd: str) -> None: ) # --- Check gh api calls to release endpoints --- - if GH_API_RELEASE_RE.match(cmd): - # If no explicit method flag, gh api defaults to GET for bare calls, - # but POST when -f/--field or --input is present. We block if a - # mutating method is specified OR if data-sending flags are present. - has_mutating_method = MUTATING_METHOD_RE.search(cmd) - has_data_flags = re.search(r"\s(-f|--field|-F|--json-field|--input)\s", cmd) - if has_mutating_method or has_data_flags: - method = "" - if has_mutating_method: - method = has_mutating_method.group(1).upper() + api = GH_API_RE.match(cmd) + if api: + method = _gh_api_mutates_release(cmd[api.end() :]) + if method: block( - f"Direct API call to release endpoint{' with ' + method + ' method' if method else ''} " + f"Direct API call to release endpoint with {method} method " f"bypasses the CI release pipeline. All mutating operations on " f"releases must go through CI.", "Use the CI release workflow to create or modify releases. " diff --git a/skills/github-release/scripts/tests/guard-gh-release-invocations.test.sh b/skills/github-release/scripts/tests/guard-gh-release-invocations.test.sh index a17342b..90c4771 100755 --- a/skills/github-release/scripts/tests/guard-gh-release-invocations.test.sh +++ b/skills/github-release/scripts/tests/guard-gh-release-invocations.test.sh @@ -183,6 +183,73 @@ check 2 'api DELETE on its own line' \ 'echo hi gh api repos/o/r/releases/1 -X DELETE' check 0 'api GET on releases' 'gh api repos/o/r/releases' +# A quoted endpoint path is how a script with variables writes it +# ("repos/$R/releases/$ID"), and the path pattern used to require the path to +# start right after the flags, so every quoted form skipped the check. +check 2 'api PATCH, path in double quotes' 'gh api -X PATCH "repos/o/r/releases/1" -f name=v1' +check 2 'api PATCH, path in single quotes' "gh api 'repos/o/r/releases/1' -X PATCH -f name=v1" +check 2 'api data flag only, quoted path' 'gh api "repos/o/r/releases/1" -f name=v1' +check 2 'api --method=PATCH, quoted path' 'gh api --method=PATCH "repos/o/r/releases/1"' +check 2 'api method in quotes' 'gh api repos/o/r/releases/1 -X "DELETE"' +check 0 'api GET, quoted path' 'gh api "repos/o/r/releases?per_page=100" --paginate --jq .' +# The call is read as argv. Before that, a flag value with a space in it ended +# the flag list early, and the long data flags were not recognised. +check 2 'api data value with a space before the path' 'gh api -f body="new notes" -X PATCH "repos/o/r/releases/1"' +check 2 'api data value with a space, no method' 'gh api -f body="new notes" "repos/o/r/releases/1"' +check 2 'api --raw-field creates a release' 'gh api repos/o/r/releases --raw-field tag_name=v1' +check 2 'api --field=value' 'gh api "repos/o/r/releases/1" --field=name=v1' +check 2 'api attached -f value' 'gh api "repos/o/r/releases/1" -fname=v1' +check 2 'api attached -X value' 'gh api -XPATCH "repos/o/r/releases/1"' +check 2 'api full URL' 'gh api -X PATCH https://api.github.com/repos/o/r/releases/1' +# shellcheck disable=SC2016 # the guard must see "$M" unexpanded +check 2 'api method from a variable' 'gh api -X "$M" repos/o/r/releases/1' +check 2 'api unbalanced quote around a release path' 'gh api -X PATCH "repos/o/r/releases/1' +check 0 'api GET with query fields' 'gh api -X GET "repos/o/r/releases" -f per_page=100' +check 0 'api jq text that mentions a method' "gh api \"repos/o/r/releases\" --jq '.[] | select(.name==\"-X POST\")'" +check 0 'api PATCH on a pull request' 'gh api -X PATCH "repos/o/r/pulls/1" -f title=x' +check 0 'api PATCH on a repo named like releases' 'gh api -X PATCH "repos/o/releases-app/pulls/1" -f title=x' +check 0 'api GET on the latest release' 'gh api repos/o/r/releases/latest' +# Owner and repository in one variable, as scripts and workflows write them. +# The first case is the command references/recovery-procedures.md prints. +# shellcheck disable=SC2016 # the guard must see the variables unexpanded +check 2 'api PATCH, owner/repo in one variable (recovery doc)' 'gh api -X PATCH "repos/$R/releases/$ID" -f name="$TAG"' +# shellcheck disable=SC2016 +check 2 'api PATCH, $GITHUB_REPOSITORY' 'gh api -X PATCH repos/$GITHUB_REPOSITORY/releases/$ID -f name=x' +# shellcheck disable=SC2016 +check 2 'api data, ${R} in braces' 'gh api "repos/${R}/releases" -f tag_name=v1' +# gh fills in {owner}/{repo} itself; the braces are part of the word. +check 2 'api DELETE, {owner}/{repo} placeholders' 'gh api -X DELETE repos/{owner}/{repo}/releases/1' +check 2 'api data, {owner}/{repo} placeholders' 'gh api repos/{owner}/{repo}/releases -f tag_name=v1' +# A "#" inside a word is not a comment in bash. +check 2 'api "#" inside a header value' 'gh api -H X-A:a#b -X DELETE repos/o/r/releases/1' +check 2 'api "#" at the end of the path' 'gh api repos/o/r/releases/1#x -X DELETE' +# Shorthand groups, read the way pflag reads them. +check 2 'api -iX DELETE' 'gh api -iX DELETE repos/o/r/releases/1' +check 2 'api -iXDELETE' 'gh api -iXDELETE repos/o/r/releases/1' +check 2 'api -if creates a release' 'gh api -if tag_name=v1 repos/o/r/releases' +check 0 'api -i on a release read' 'gh api -i repos/o/r/releases/latest' +# A shell comment is cut the way bash cuts it: words after it never reach gh. +check 2 'api DELETE with a comment naming GET' 'gh api repos/o/r/releases/1 -X DELETE # -X GET' +check 2 'api data with a comment naming GET' 'gh api repos/o/r/releases -f tag_name=v1 # -X GET' +# The call is also judged without the comment cut, so a release path that +# appears only in a comment blocks. That is the safe direction: the cutter is a +# text scan, and a "#" bash does not read as a comment must never cost a real +# method (the two cases below). +check 2 'api POST elsewhere, release path only in a comment' 'gh api repos/o/r/issues -X POST # repos/o/r/releases' +# shellcheck disable=SC2016 # the guard must see the expansion unexpanded +check 2 'api "#" inside a ${...} expansion' 'gh api repos/o/r/releases/1 -H X-A:${V/ #/} -X DELETE' +check 2 "api \"#\" after a \$'...' string with an escaped quote" "gh api repos/o/r/releases/1 -H \$'X-A: a\\'b' -H 'X-B: #c' -X DELETE" +check 2 'api method flag without a value' 'gh api repos/o/r/releases/1 -X DELETE -X' +check 2 'api numeric repositories route' 'gh api repositories/123/releases/1 -X DELETE' +# shellcheck disable=SC2016 # the guard must see the expansion unexpanded +check 2 'create after a ${...} assignment with a blank' 'a=${X:-foo bar} gh release create v1.2.3' +check 2 'create after a double-quoted assignment with a blank' 'A="x y" gh release create v1.2.3' +check 2 'create after a single-quoted assignment with a blank' "A='x y' gh release create v1.2.3" +check 2 'create after an escaped blank in an assignment' 'A=x\ y gh release create v1.2.3' +check 2 'api DELETE after a quoted assignment with a blank' 'A="x y" gh api -X DELETE repos/o/r/releases/1' +check 0 'view after a quoted assignment with a blank' 'A="x y" gh release view v1.2.3' +# shellcheck disable=SC2016 +check 2 'create in a ${ cmd; } substitution' 'echo ${ gh release create v1.2.3; }' if [[ "$fail" == 0 ]]; then printf '\nAll gh-release invocation tests passed\n' diff --git a/skills/github-release/scripts/tests/guard-tag-invocations.test.sh b/skills/github-release/scripts/tests/guard-tag-invocations.test.sh index 795e8ed..57697da 100755 --- a/skills/github-release/scripts/tests/guard-tag-invocations.test.sh +++ b/skills/github-release/scripts/tests/guard-tag-invocations.test.sh @@ -160,6 +160,20 @@ check 0 'the command name inside a commit message' 'git commit -m "git tag v1.2. check 2 'lightweight tag in a subshell' '(git tag v1.2.3)' check 2 'lightweight tag in a subshell after a listing' 'git tag -l && (git tag v1.2.3)' check 2 'lightweight tag in a brace group' '{ git tag v1.2.3; }' +check 2 'lightweight tag in a brace group, no blank before }' '{ git tag v1.2.3;}' +# A brace inside a word is text. An assignment whose ${...} holds a blank +# still counts as a prefix, and bash 5.3's ${ cmd; } still opens a command. +# shellcheck disable=SC2016 # the guard must see the expansions unexpanded +check 2 'lightweight tag after a ${...} assignment with a blank' 'a=${X:-foo bar} git tag v1.2.3' +# shellcheck disable=SC2016 +check 2 'lightweight tag after a ${//} assignment' 'a=${X// /_} git tag v1.2.3' +check 2 'lightweight tag after a double-quoted assignment with a blank' 'a="x y" git tag v1.2.3' +check 2 'lightweight tag after a single-quoted assignment with a blank' "a='x y' git tag v1.2.3" +check 2 'lightweight tag after an escaped blank in an assignment' 'a=x\ y git tag v1.2.3' +check 0 'signed tag after a quoted assignment with a blank' 'a="x y" git tag -s v1.2.3 -m v1.2.3' +# shellcheck disable=SC2016 +check 2 'lightweight tag in a ${ cmd; } substitution' 'echo ${ git tag v1.2.3; }' +check 0 'braces inside words do not make a command' 'echo {a,b} x}{ git-tag' # The single quotes are the point: the guard has to receive the substitution # as literal text, exactly as the harness would hand it over. # shellcheck disable=SC2016