From 1bb59b6a57165658f5a7d654d12e40fa66e0cfb8 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 04:01:31 +0000 Subject: [PATCH 1/2] validate.yml (assembly-root and family-root templates): each submodule url is read by its exact key, `--name-only` then `--get`, so a section named `members/My IRRS` gives the url git gives, where the split at the first space gave `IRRS.url `, left that repository out of the App token and listed it wrong in both failure steps (#741) The templates read `.gitmodules` three times each, and each read split `git config --get-regexp '\.url$'`'s `key value` lines at the first space (`| cut -d' ' -f2-`, `| while read -r _ url`). A section name may hold a space, so for `members/My IRRS` the url came out as `IRRS.url `: the names step matched it to neither `https://github.com/` nor `git@github.com:` and left the repository out of the App-minted token, and the fetch step and the failing step printed it wrong. #700 made the doctor read such a name; CI now agrees with it. Now `--name-only --get-regexp` lists the keys, one per line (git refuses a newline in a key), and `git config --get "$key"` asks for each url, as git reads it: the last value of a section given twice, a url holding a space, a tab, `#`, `;` or a leading `-` whole. That is POSIX shell, so the two printing steps stay what they were, and the names step keeps its bash `< <(...)`. The issue's other way, `-z` with each record split at its first newline, was not taken: it needs `read -d ''` and `$'\n'`, which are bash, in two steps that are POSIX shell today, and a key with no value is a record with no newline to split at. tests/test_validate_workflow_gitmodules_urls.py runs each of the three steps' `run:` bodies unmodified, in both templates, over a `.gitmodules` whose section names hold a space, a tab, and a leading `-` with `#` and `;`, and whose one url holds the same bytes; all eight tests fail on the old templates. Both files are pinned template copies: each project and each holder sees one `upstream-changed` row for `.github/workflows/validate.yml`. Co-Authored-By: Claude Sonnet 5.5 --- .../.github/workflows/validate.yml | 19 ++- .../.github/workflows/validate.yml | 19 ++- .../test_validate_workflow_gitmodules_urls.py | 152 ++++++++++++++++++ 3 files changed, 182 insertions(+), 8 deletions(-) create mode 100644 tests/test_validate_workflow_gitmodules_urls.py diff --git a/templates/assembly-root/.github/workflows/validate.yml b/templates/assembly-root/.github/workflows/validate.yml index 4641aa9c..0ce8a0f4 100644 --- a/templates/assembly-root/.github/workflows/validate.yml +++ b/templates/assembly-root/.github/workflows/validate.yml @@ -56,6 +56,14 @@ jobs: # failing) on any that don't, since those can only ever be reached by # `SHAPE_LEGS_TOKEN`. Runs only when the App secrets are present — its # only consumer is the mint step below, which is guarded the same way. + # + # Each url is read by its exact key: `--name-only` lists the keys, then + # `--get` asks for one, as git reads it. Splitting `--get-regexp`'s + # `key value` lines at the first space gave a section named `members/My + # IRRS` the url `IRRS.url `, which matched neither form and dropped + # that leg from the token (#741). A key holds no line break, so one per + # line is exact; the two steps below that print the urls read them the + # same way. - name: leg repository names from .gitmodules id: leg-names if: env.SHAPE_LEGS_APP_SET == 'true' @@ -65,7 +73,8 @@ jobs: set -e legs="" if [ -f .gitmodules ]; then - while IFS= read -r url; do + while IFS= read -r key; do + url=$(git config --file .gitmodules --get "$key") || continue [ -n "$url" ] || continue case "$url" in https://github.com/*) repo="${url#https://github.com/}" ;; @@ -84,7 +93,7 @@ jobs: else echo "::warning::leg $owner/$name is owned by a different account ($owner) than this repository ($REPO_OWNER); a GitHub App installation token is per-owner, so it is excluded from the App-minted token here and can only be reached via SHAPE_LEGS_TOKEN." fi - done < <(git config --file .gitmodules --get-regexp '\.url$' | cut -d' ' -f2- || true) + done < <(git config --file .gitmodules --name-only --get-regexp '\.url$' || true) fi echo "legs=$legs" >> "$GITHUB_OUTPUT" echo "legs for the App-minted token: $legs" @@ -170,7 +179,8 @@ jobs: else echo "legs_available=false" >> "$GITHUB_OUTPUT" echo "::warning::submodule checkout failed (git exit $status) using credential source '$legs_credential'. If the legs are PRIVATE or internal sibling repositories, the default GITHUB_TOKEN cannot read them. Configure a GitHub App (SHAPE_LEGS_APP_ID + SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or set SHAPE_LEGS_TOKEN — a fine-grained PAT — with contents:read on the legs, as a repository or organisation secret. Leg URLs:" - git config --file .gitmodules --get-regexp '\.url$' | while read -r _ url; do + git config --file .gitmodules --name-only --get-regexp '\.url$' | while IFS= read -r key; do + url=$(git config --file .gitmodules --get "$key") || continue echo "::warning:: $url" done fi @@ -217,7 +227,8 @@ jobs: if: steps.submodules.outputs.legs_available != 'true' && (env.SHAPE_LEGS_TOKEN_SET == 'true' || env.SHAPE_LEGS_APP_SET == 'true') run: | echo "::error::A leg credential is configured (source: ${{ steps.submodules.outputs.legs_credential }}) but 'git submodule update --init --recursive' still failed. The root checkout does not use this token — it reads with the workflow's default token — so this means the '${{ steps.submodules.outputs.legs_credential }}' credential cannot read one or more of the following LEG repositories (wrong token/App scope, a missing contents:read grant, an incomplete App installation, or a leg URL problem):" - git config --file .gitmodules --get-regexp '\.url$' | while read -r _ url; do + git config --file .gitmodules --name-only --get-regexp '\.url$' | while IFS= read -r key; do + url=$(git config --file .gitmodules --get "$key") || continue echo "::error:: $url" done exit 1 diff --git a/templates/family-root/.github/workflows/validate.yml b/templates/family-root/.github/workflows/validate.yml index 4d2279d6..ab6c620a 100644 --- a/templates/family-root/.github/workflows/validate.yml +++ b/templates/family-root/.github/workflows/validate.yml @@ -52,6 +52,14 @@ jobs: # an `owner/name` pair, keeps the names whose owner matches # `github.repository_owner`, and warns — excluding, not failing — on any # that do not, since those can only be reached by SHAPE_LEGS_TOKEN. + # + # Each url is read by its exact key: `--name-only` lists the keys, then + # `--get` asks for one, as git reads it. Splitting `--get-regexp`'s + # `key value` lines at the first space gave a section named `members/My + # IRRS` the url `IRRS.url `, which matched neither form and dropped + # that member from the token (#741). A key holds no line break, so one + # per line is exact; the two steps below that print the urls read them + # the same way. - name: member repository names from .gitmodules id: member-names if: env.SHAPE_LEGS_APP_SET == 'true' @@ -61,7 +69,8 @@ jobs: set -e members="" if [ -f .gitmodules ]; then - while IFS= read -r url; do + while IFS= read -r key; do + url=$(git config --file .gitmodules --get "$key") || continue [ -n "$url" ] || continue case "$url" in https://github.com/*) repo="${url#https://github.com/}" ;; @@ -80,7 +89,7 @@ jobs: else echo "::warning::member $owner/$name is owned by a different account ($owner) than this family ($REPO_OWNER); a GitHub App installation token is per-owner, so it is excluded from the App-minted token here and can only be reached via SHAPE_LEGS_TOKEN." fi - done < <(git config --file .gitmodules --get-regexp '\.url$' | cut -d' ' -f2- || true) + done < <(git config --file .gitmodules --name-only --get-regexp '\.url$' || true) fi echo "members=$members" >> "$GITHUB_OUTPUT" echo "members for the App-minted token: $members" @@ -171,7 +180,8 @@ jobs: else echo "members_available=false" >> "$GITHUB_OUTPUT" echo "::warning::submodule checkout failed (git exit $status) using credential source '$members_credential'. If the members are PRIVATE or internal sibling repositories, the default GITHUB_TOKEN cannot read them. Configure a GitHub App (SHAPE_LEGS_APP_ID + SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or set SHAPE_LEGS_TOKEN — a fine-grained PAT — with contents:read on the members and their legs. Member URLs:" - git config --file .gitmodules --get-regexp '\.url$' | while read -r _ url; do + git config --file .gitmodules --name-only --get-regexp '\.url$' | while IFS= read -r key; do + url=$(git config --file .gitmodules --get "$key") || continue echo "::warning:: $url" done fi @@ -211,7 +221,8 @@ jobs: if: steps.submodules.outputs.members_available != 'true' && (env.SHAPE_LEGS_TOKEN_SET == 'true' || env.SHAPE_LEGS_APP_SET == 'true') run: | echo "::error::A member credential is configured (source: ${{ steps.submodules.outputs.members_credential }}) but 'git submodule update --init --recursive' still failed. The root checkout does not use this token — it reads with the workflow's default token — so this means the '${{ steps.submodules.outputs.members_credential }}' credential cannot read one or more of the following MEMBER repositories, or a leg of one (wrong token/App scope, a missing contents:read grant, an incomplete App installation, or a URL problem):" - git config --file .gitmodules --get-regexp '\.url$' | while read -r _ url; do + git config --file .gitmodules --name-only --get-regexp '\.url$' | while IFS= read -r key; do + url=$(git config --file .gitmodules --get "$key") || continue echo "::error:: $url" done exit 1 diff --git a/tests/test_validate_workflow_gitmodules_urls.py b/tests/test_validate_workflow_gitmodules_urls.py new file mode 100644 index 00000000..67360712 --- /dev/null +++ b/tests/test_validate_workflow_gitmodules_urls.py @@ -0,0 +1,152 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The `validate.yml` templates read each url out of `.gitmodules` as git +does (#741). + +Both templates (`templates/assembly-root/` and `templates/family-root/`) read +the urls three times: the step that names the repositories the GitHub App +token is minted for, the step that fetches the submodules and lists the urls +where it cannot, and the step that fails the job and lists them again. Each +used to split `git config --get-regexp '\\.url$'`'s `key value` lines at the +first space, so a section named `members/My IRRS` gave the url `IRRS.url +`: the names step matched it to neither form and left the repository out +of the token, and the other two printed it. + +Like `tests/test_scaffold_e2e.py::test_the_leg_name_extraction_...`, these run +each step's `run:` body UNMODIFIED, as the real bash it is, so the shell is +what is tested and not a Python reading of it. The one thing a runner does +that a test cannot is reach the legs: `git submodule update` is a stub that +exits 1, which is how the two printing steps are reached, and every other +`git` is the real one. +""" + +from __future__ import annotations + +import os +import re +import shutil +import subprocess +from pathlib import Path + +import pytest + +from conftest import REPO, WINDOWS_SKIP, git +from test_scaffold_e2e import _step_run_script + +#: What each template calls its steps. The names step's output is `legs=` in +#: the assembly root and `members=` in the family root, which names members. +TEMPLATES = { + "assembly-root": { + "names": "id: leg-names", + "output": "legs", + "fetch": "id: submodules", + "fail": "fail — a leg credential is configured but the legs still", + }, + "family-root": { + "names": "id: member-names", + "output": "members", + "fetch": "id: submodules", + "fail": "fail — a credential is configured but the members still", + }, +} +EACH_TEMPLATE = pytest.mark.parametrize("template", sorted(TEMPLATES)) + +#: Section name -> url, in the order the file lists them. The first three are +#: names git allows and `--get-regexp`'s `key value` line cannot hold: a +#: space, a tab, and a leading `-` with `#` and `;`. The fourth is a url, not +#: a name, with the same bytes in it (a leading `-`, a space, a tab, `#`, `;` +#: and a trailing space); it names no repository, so the names step skips it. +SECTIONS = { + "members/My IRRS": "https://github.com/acme/IRRS.git", + "tab\there": "git@github.com:acme/Tabbed.git", + "-lead #hash;semi": "https://github.com/acme/Odd.git", + "plain": "-odd url\twith #hash;semi ", +} +THE_LEGS = "IRRS,Tabbed,Odd" + + +def workflow_text(template: str) -> str: + path = (REPO / "templates" / template / ".github" / "workflows" + / "validate.yml") + return path.read_text(encoding="utf-8") + + +@pytest.fixture +def holder(tmp_path: Path) -> Path: + """A folder whose `.gitmodules` is `SECTIONS`, written by git so that its + quoting is git's.""" + work = tmp_path / "holder" + work.mkdir() + for name, url in SECTIONS.items(): + git("config", "--file", ".gitmodules", f"submodule.{name}.url", url, + cwd=work) + return work + + +def run_step(template: str, step: str, holder: Path, tmp_path: Path): + """`(completed process, the step's $GITHUB_OUTPUT)` for one step's `run:` + body in `holder`, under `bash -e` as the runner starts it. + + The `${{ ... }}` expressions are the runner's to fill in, and bash reads + them as a bad substitution, so each becomes a word. + """ + spec = TEMPLATES[template] + body = _step_run_script(workflow_text(template), spec[step]) + script = re.sub(r"\$\{\{.*?\}\}", "EXPRESSION", body) + stubs = tmp_path / "stubs" + stubs.mkdir() + stub = stubs / "git" + stub.write_bytes(b'#!/bin/sh\n[ "$1" = submodule ] && exit 1\n' + b'exec "$REAL_GIT" "$@"\n') + stub.chmod(0o755) + output = tmp_path / "github_output" + output.write_bytes(b"") + env = {**os.environ, "REAL_GIT": shutil.which("git"), + "PATH": str(stubs) + os.pathsep + os.environ["PATH"], + "GITHUB_OUTPUT": str(output), "REPO_OWNER": "acme"} + result = subprocess.run(["bash", "-e", "-c", script], cwd=holder, env=env, + capture_output=True, text=True, encoding="utf-8", + check=False) + return result, output.read_text(encoding="utf-8") + + +@WINDOWS_SKIP # it runs the step's `run:` body as the REAL bash it is +@EACH_TEMPLATE +def test_the_names_step_keeps_a_repository_whose_section_name_is_not_a_word( + template, holder, tmp_path): + result, output = run_step(template, "names", holder, tmp_path) + assert result.returncode == 0, result.stderr + output_key = TEMPLATES[template]["output"] + assert output.splitlines() == [f"{output_key}={THE_LEGS}"] + + +@WINDOWS_SKIP # it runs the step's `run:` body as the REAL bash it is +@EACH_TEMPLATE +def test_the_fetch_step_lists_each_url_as_it_is_when_the_fetch_fails( + template, holder, tmp_path): + result, _ = run_step(template, "fetch", holder, tmp_path) + assert result.returncode == 0, result.stderr + listed = [line for line in result.stdout.split("\n") + if line.startswith("::warning:: ")] + assert listed == [f"::warning:: {url}" for url in SECTIONS.values()] + + +@WINDOWS_SKIP # it runs the step's `run:` body as the REAL bash it is +@EACH_TEMPLATE +def test_the_failing_step_lists_each_url_as_it_is(template, holder, tmp_path): + result, _ = run_step(template, "fail", holder, tmp_path) + assert result.returncode == 1, result.stderr + listed = [line for line in result.stdout.split("\n") + if line.startswith("::error:: ")] + assert listed == [f"::error:: {url}" for url in SECTIONS.values()] + + +@EACH_TEMPLATE +def test_no_step_reads_a_listing_that_a_section_name_can_split(template): + """The class of defect, not only its three instances: `--get-regexp` + without `--name-only` prints `key value` lines, and a line holding a + section name with a space cannot be split back into the two.""" + reads = [line for line in workflow_text(template).splitlines() + if "--get-regexp" in line and not line.lstrip().startswith("#")] + assert reads + for line in reads: + assert "--name-only" in line, line From 67324e915b09c2674a45ad529990d97c417358aa Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 04:30:47 +0000 Subject: [PATCH 2/2] validate.yml (assembly-root and family-root templates): a url holding a line break or a carriage return is skipped, each repository is named once, and each url is read with `--get --`, so a url cannot print a workflow command of its own, `legs` and `members` hold no repository twice, and `[-x] url` is read without git's usage dump (#741) Round 2 of #782, from RV-c's review. - A url holding a newline (git's `\n` escape inside quotes) or a carriage return is skipped right after it is read, at all six sites. Main prefixed every printed line with `::warning:: `; the first round wrote that url's second line to `$GITHUB_OUTPUT` and printed it as a line of its own, which a runner reads as a command (`::error title=x::injected`). git cannot clone such a url. - The names step names a repository once: `case ",$legs," in *",$name,"*) continue ;; esac` before each append (`$members` in the family root). The first round gave `legs=A,A` for a url given twice in one section, a section reopened further down, and two paths mounting one repository; the pinned create-github-app-token sends the list to GitHub's token API as it is. - `--get -- "$key"`. `--get-regexp '\.url$'` lists any section's url key, not only `submodule.*`, and `[-x] url = ...` makes `git config --get -x.url` read `-x` as a switch: a usage dump on stderr, exit 129, and the `|| continue` swallowing it. The first round's note that every key starts `submodule.` was wrong. tests/test_validate_workflow_gitmodules_urls.py: the shared `.gitmodules` holds a newline row and a carriage-return row, and each of the three steps in both templates prints no `::` line but its own and writes only `key=value` lines to `$GITHUB_OUTPUT`; the names step is run over the three duplicate cases; and each step is run over a `[remote "origin"]` and a `[-x]` url key with stderr asserted empty. Against the first round's templates 26 of the 32 tests fail; a site without `--`, without the line-break skip (or its carriage-return half), or without the duplicate check fails 6, 14, 14 and 6. Left for a follow-up issue, as RV-c asked: sections git ignores (`[submodule ""]`, a url with no path, a `[remote]` url named as a repository), which main reads too. Co-Authored-By: Claude Sonnet 5.5 --- .../.github/workflows/validate.yml | 21 ++++- .../.github/workflows/validate.yml | 21 ++++- .../test_validate_workflow_gitmodules_urls.py | 93 ++++++++++++++++++- 3 files changed, 124 insertions(+), 11 deletions(-) diff --git a/templates/assembly-root/.github/workflows/validate.yml b/templates/assembly-root/.github/workflows/validate.yml index 0ce8a0f4..8ad2ee05 100644 --- a/templates/assembly-root/.github/workflows/validate.yml +++ b/templates/assembly-root/.github/workflows/validate.yml @@ -63,7 +63,10 @@ jobs: # IRRS` the url `IRRS.url `, which matched neither form and dropped # that leg from the token (#741). A key holds no line break, so one per # line is exact; the two steps below that print the urls read them the - # same way. + # same way. A url holding a line break or a carriage return is skipped + # in all three: it could print a workflow command of its own, and git + # cannot clone it. And each repository is named once, however many + # sections or paths give it. - name: leg repository names from .gitmodules id: leg-names if: env.SHAPE_LEGS_APP_SET == 'true' @@ -74,7 +77,10 @@ jobs: legs="" if [ -f .gitmodules ]; then while IFS= read -r key; do - url=$(git config --file .gitmodules --get "$key") || continue + url=$(git config --file .gitmodules --get -- "$key") || continue + # A url holding a line break could print a workflow command of its own. + case $url in *' + '*|*"$(printf '\r')"*) continue ;; esac [ -n "$url" ] || continue case "$url" in https://github.com/*) repo="${url#https://github.com/}" ;; @@ -85,6 +91,7 @@ jobs: owner="${repo%%/*}" name="${repo#*/}" if [ "$owner" = "$REPO_OWNER" ]; then + case ",$legs," in *",$name,"*) continue ;; esac if [ -z "$legs" ]; then legs="$name" else @@ -180,7 +187,10 @@ jobs: echo "legs_available=false" >> "$GITHUB_OUTPUT" echo "::warning::submodule checkout failed (git exit $status) using credential source '$legs_credential'. If the legs are PRIVATE or internal sibling repositories, the default GITHUB_TOKEN cannot read them. Configure a GitHub App (SHAPE_LEGS_APP_ID + SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or set SHAPE_LEGS_TOKEN — a fine-grained PAT — with contents:read on the legs, as a repository or organisation secret. Leg URLs:" git config --file .gitmodules --name-only --get-regexp '\.url$' | while IFS= read -r key; do - url=$(git config --file .gitmodules --get "$key") || continue + url=$(git config --file .gitmodules --get -- "$key") || continue + # A url holding a line break could print a workflow command of its own. + case $url in *' + '*|*"$(printf '\r')"*) continue ;; esac echo "::warning:: $url" done fi @@ -228,7 +238,10 @@ jobs: run: | echo "::error::A leg credential is configured (source: ${{ steps.submodules.outputs.legs_credential }}) but 'git submodule update --init --recursive' still failed. The root checkout does not use this token — it reads with the workflow's default token — so this means the '${{ steps.submodules.outputs.legs_credential }}' credential cannot read one or more of the following LEG repositories (wrong token/App scope, a missing contents:read grant, an incomplete App installation, or a leg URL problem):" git config --file .gitmodules --name-only --get-regexp '\.url$' | while IFS= read -r key; do - url=$(git config --file .gitmodules --get "$key") || continue + url=$(git config --file .gitmodules --get -- "$key") || continue + # A url holding a line break could print a workflow command of its own. + case $url in *' + '*|*"$(printf '\r')"*) continue ;; esac echo "::error:: $url" done exit 1 diff --git a/templates/family-root/.github/workflows/validate.yml b/templates/family-root/.github/workflows/validate.yml index ab6c620a..56c5dbef 100644 --- a/templates/family-root/.github/workflows/validate.yml +++ b/templates/family-root/.github/workflows/validate.yml @@ -59,7 +59,10 @@ jobs: # IRRS` the url `IRRS.url `, which matched neither form and dropped # that member from the token (#741). A key holds no line break, so one # per line is exact; the two steps below that print the urls read them - # the same way. + # the same way. A url holding a line break or a carriage return is + # skipped in all three: it could print a workflow command of its own, + # and git cannot clone it. And each member is named once, however many + # sections or paths give it. - name: member repository names from .gitmodules id: member-names if: env.SHAPE_LEGS_APP_SET == 'true' @@ -70,7 +73,10 @@ jobs: members="" if [ -f .gitmodules ]; then while IFS= read -r key; do - url=$(git config --file .gitmodules --get "$key") || continue + url=$(git config --file .gitmodules --get -- "$key") || continue + # A url holding a line break could print a workflow command of its own. + case $url in *' + '*|*"$(printf '\r')"*) continue ;; esac [ -n "$url" ] || continue case "$url" in https://github.com/*) repo="${url#https://github.com/}" ;; @@ -81,6 +87,7 @@ jobs: owner="${repo%%/*}" name="${repo#*/}" if [ "$owner" = "$REPO_OWNER" ]; then + case ",$members," in *",$name,"*) continue ;; esac if [ -z "$members" ]; then members="$name" else @@ -181,7 +188,10 @@ jobs: echo "members_available=false" >> "$GITHUB_OUTPUT" echo "::warning::submodule checkout failed (git exit $status) using credential source '$members_credential'. If the members are PRIVATE or internal sibling repositories, the default GITHUB_TOKEN cannot read them. Configure a GitHub App (SHAPE_LEGS_APP_ID + SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or set SHAPE_LEGS_TOKEN — a fine-grained PAT — with contents:read on the members and their legs. Member URLs:" git config --file .gitmodules --name-only --get-regexp '\.url$' | while IFS= read -r key; do - url=$(git config --file .gitmodules --get "$key") || continue + url=$(git config --file .gitmodules --get -- "$key") || continue + # A url holding a line break could print a workflow command of its own. + case $url in *' + '*|*"$(printf '\r')"*) continue ;; esac echo "::warning:: $url" done fi @@ -222,7 +232,10 @@ jobs: run: | echo "::error::A member credential is configured (source: ${{ steps.submodules.outputs.members_credential }}) but 'git submodule update --init --recursive' still failed. The root checkout does not use this token — it reads with the workflow's default token — so this means the '${{ steps.submodules.outputs.members_credential }}' credential cannot read one or more of the following MEMBER repositories, or a leg of one (wrong token/App scope, a missing contents:read grant, an incomplete App installation, or a URL problem):" git config --file .gitmodules --name-only --get-regexp '\.url$' | while IFS= read -r key; do - url=$(git config --file .gitmodules --get "$key") || continue + url=$(git config --file .gitmodules --get -- "$key") || continue + # A url holding a line break could print a workflow command of its own. + case $url in *' + '*|*"$(printf '\r')"*) continue ;; esac echo "::error:: $url" done exit 1 diff --git a/tests/test_validate_workflow_gitmodules_urls.py b/tests/test_validate_workflow_gitmodules_urls.py index 67360712..5dfb96d1 100644 --- a/tests/test_validate_workflow_gitmodules_urls.py +++ b/tests/test_validate_workflow_gitmodules_urls.py @@ -17,6 +17,12 @@ that a test cannot is reach the legs: `git submodule update` is a stub that exits 1, which is how the two printing steps are reached, and every other `git` is the real one. + +Three things beside the first-space split, from the review of the fix: a url +holding a line break or a carriage return is skipped in all three steps, since +printed it could be a workflow command of its own; each repository is named +once however many sections or paths give it; and a url key outside a +`submodule` section, even `[-x] url`, is read without git's usage dump. """ from __future__ import annotations @@ -62,6 +68,33 @@ "plain": "-odd url\twith #hash;semi ", } THE_LEGS = "IRRS,Tabbed,Odd" +#: Urls the steps skip, given by git's `\n` escape and a raw carriage return: +#: each holds a line that a runner would read as a workflow command. +LINE_BREAKS = { + "nl": "https://github.com/acme/A.git\n::error title=x::injected", + "cr": "https://github.com/acme/B.git\r::error title=y::injected", +} +#: What a step prints of its own: its summary line, and the ` ` lines. +OWN_COMMAND = re.compile(r"::(warning|error)::( |submodule checkout failed|A )") +EACH_STEP = pytest.mark.parametrize("step", ["names", "fetch", "fail"]) + + +def acme(name: str, scheme: str = "https://github.com/") -> str: + return f"url = {scheme}acme/{name}.git\n" + + +#: Each case a repository could be named twice, as a `.gitmodules` text and +#: the repositories the names step names for it: once each, first seen first. +DUPLICATES = { + "a url given twice in one section": ( + '[submodule "m"]\n\t' + acme("Stale") + "\t" + acme("A"), "A"), + "a section reopened further down": ( + '[submodule "m"]\n\t' + acme("Stale") + '[submodule "n"]\n\t' + + acme("B") + '[submodule "m"]\n\t' + acme("A"), "A,B"), + "two paths mounting one repository": ( + '[submodule "m"]\n\t' + acme("A") + '[submodule "n"]\n\t' + + acme("A", "git@github.com:"), "A"), +} def workflow_text(template: str) -> str: @@ -72,16 +105,24 @@ def workflow_text(template: str) -> str: @pytest.fixture def holder(tmp_path: Path) -> Path: - """A folder whose `.gitmodules` is `SECTIONS`, written by git so that its - quoting is git's.""" + """A folder whose `.gitmodules` is `SECTIONS` and then `LINE_BREAKS`, + written by git so that its quoting is git's.""" work = tmp_path / "holder" work.mkdir() - for name, url in SECTIONS.items(): + for name, url in {**SECTIONS, **LINE_BREAKS}.items(): git("config", "--file", ".gitmodules", f"submodule.{name}.url", url, cwd=work) return work +def holder_of(tmp_path: Path, gitmodules: str) -> Path: + """A folder whose `.gitmodules` is `gitmodules`, as written.""" + work = tmp_path / "holder" + work.mkdir() + (work / ".gitmodules").write_bytes(gitmodules.encode("utf-8")) + return work + + def run_step(template: str, step: str, holder: Path, tmp_path: Path): """`(completed process, the step's $GITHUB_OUTPUT)` for one step's `run:` body in `holder`, under `bash -e` as the runner starts it. @@ -150,3 +191,49 @@ def test_no_step_reads_a_listing_that_a_section_name_can_split(template): assert reads for line in reads: assert "--name-only" in line, line + + +@WINDOWS_SKIP # it runs the step's `run:` body as the REAL bash it is +@EACH_STEP +@EACH_TEMPLATE +def test_a_url_holding_a_line_break_prints_no_workflow_command_of_its_own( + template, step, holder, tmp_path): + result, _ = run_step(template, step, holder, tmp_path) + commands = [line for line in result.stdout.split("\n") + if line.startswith("::") and not OWN_COMMAND.match(line)] + assert commands == [] + + +@WINDOWS_SKIP # it runs the step's `run:` body as the REAL bash it is +@EACH_STEP +@EACH_TEMPLATE +def test_a_url_holding_a_line_break_writes_only_key_value_lines_to_the_output( + template, step, holder, tmp_path): + _, output = run_step(template, step, holder, tmp_path) + stray = [line for line in output.split("\n") + if line and not re.match(r"[a-z_]+=", line)] + assert stray == [] + + +@WINDOWS_SKIP # it runs the step's `run:` body as the REAL bash it is +@pytest.mark.parametrize("case", sorted(DUPLICATES)) +@EACH_TEMPLATE +def test_the_names_step_names_a_repository_once(template, case, tmp_path): + gitmodules, names = DUPLICATES[case] + work = holder_of(tmp_path, gitmodules) + _, output = run_step(template, "names", work, tmp_path) + assert output.splitlines() == [f"{TEMPLATES[template]['output']}={names}"] + + +@WINDOWS_SKIP # it runs the step's `run:` body as the REAL bash it is +@EACH_STEP +@EACH_TEMPLATE +def test_a_url_key_outside_a_submodule_section_is_read_without_a_usage_dump( + template, step, tmp_path): + """`--get-regexp '\\.url$'` lists any section's url, and `git config + --get -x.url` reads `-x` as a switch: a usage dump on stderr, exit 129, + and the url swallowed by the `|| continue` after it.""" + work = holder_of(tmp_path, '[remote "origin"]\n\t' + acme("Remote") + + "[-x]\n\t" + acme("Dash")) + result, _ = run_step(template, step, work, tmp_path) + assert result.stderr == ""