-
Notifications
You must be signed in to change notification settings - Fork 0
fix(strix): accept legal Packrat fixture paths #800
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
af5516c
test(ci): materialize legal Strix path regression
seonghobae f3a8640
fix(ci): repair legal Strix path materializer
seonghobae 4e1e78e
fix(strix): accept legal Packrat fixture paths
abd2ffc
ci(strix): verify legal changed-path policy
seonghobae 5e46cbb
fix(ci): use setup-python without cache input
seonghobae 0007677
test(strix): reject embedded traversal components
seonghobae c2333af
test(ci): materialize embedded traversal repair
seonghobae aa433fb
fix(ci): repair traversal materializer contract
seonghobae f1eabf9
chore(ci): remove failed traversal materializer
seonghobae 1fd32ee
chore(ci): remove workflow-token repair attempt
seonghobae 69f3671
ci(strix): run full exact-head path contract
seonghobae aabdd64
ci(strix): materialize non-workflow traversal repair
seonghobae 7338c79
fix(strix): reject raw traversal components
d9081cb
chore(ci): remove successful traversal materializer
seonghobae 4070b81
test(strix): require hash-verified workflow dependencies
seonghobae 23479a9
fix(strix): hash-pin exact workflow wheels
seonghobae 5661987
docs(doctoring): record Strix wheel hash boundary
seonghobae c909cea
test(strix): require hash file installation contract
seonghobae c51bf92
fix(strix): install hash pins from requirements file
seonghobae 87c302c
chore(stack): reconcile legal Strix path repair with main
seonghobae 70bdd49
test(strix): require complete shell regression suite
seonghobae bb28a1f
fix(strix): execute complete shell regression suite
seonghobae 8c9c034
test(strix): require manual quality dispatch
seonghobae 138257a
fix(strix): expose permanent manual quality run
seonghobae 0e43bde
test(strix): preserve default-branch-only workflow source
seonghobae 4ffa1dd
fix(strix): keep manual branch execution disabled
seonghobae 48ebdff
docs(strix): record trusted workflow source boundary
seonghobae fccb28b
test(strix): expose manual-dispatch key spelling gap
seonghobae dc7748a
test(strix): harden manual-dispatch YAML key guard
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| name: Strix Changed Path Quality CI | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: [main] | ||
| paths: | ||
| - ".github/workflows/strix-changed-path-quality-ci.yml" | ||
| - "CHANGELOG.md" | ||
| - "docs/doctoring/strix-legal-git-paths.md" | ||
| - "scripts/ci/strix_quick_gate.sh" | ||
| - "scripts/ci/test_strix_quick_gate.sh" | ||
| - "tests/test_strix_changed_path_policy.py" | ||
| - "tests/test_strix_workflow_dependency_hashes.py" | ||
|
seonghobae marked this conversation as resolved.
|
||
|
|
||
|
seonghobae marked this conversation as resolved.
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| concurrency: | ||
| group: strix-changed-path-quality-${{ github.event.pull_request.number || github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| exact-head-path-policy: | ||
| if: github.event_name != 'pull_request' || github.event.action != 'closed' | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 10 | ||
| steps: | ||
| - name: Checkout exact source revision | ||
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.sha || github.sha }} | ||
| persist-credentials: false | ||
|
|
||
| - name: Set up Python | ||
| uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 | ||
| with: | ||
| python-version: "3.14" | ||
|
|
||
| - name: Install exact hash-verified test runner dependencies | ||
| env: | ||
| PIP_DISABLE_PIP_VERSION_CHECK: "1" | ||
| PIP_NO_INPUT: "1" | ||
| shell: bash --noprofile --norc -e -o pipefail {0} | ||
| run: | | ||
| cat >"${RUNNER_TEMP}/strix-quality-requirements.txt" <<'EOF' | ||
| coverage==7.15.2 --hash=sha256:b9a6367e4aff723e8ee8190836836124284e8fcd4265e307c844010cfa074f3f | ||
| iniconfig==2.1.0 --hash=sha256:9deba5723312380e77435581c6bf4935c94cbfab9b1ed33ef8d238ea168eb760 | ||
| packaging==26.2 --hash=sha256:5fc45236b9446107ff2415ce77c807cee2862cb6fac22b8a73826d0693b0980e | ||
| pluggy==1.6.0 --hash=sha256:e920276dd6813095e9377c0bc5566d94c932c33b27a3e3945d8389c374dd4746 | ||
| pygments==2.20.0 --hash=sha256:81a9e26dd42fd28a23a2d169d86d7ac03b46e2f8b59ed4698fb4785f946d0176 | ||
| pytest==9.1.1 --hash=sha256:37a86b45efb9a47a61a36449063e8e18d0cab3161329fc099eb21783169c4f0c | ||
| EOF | ||
| python -m pip install \ | ||
| --only-binary=:all: \ | ||
| --require-hashes \ | ||
| -r "${RUNNER_TEMP}/strix-quality-requirements.txt" | ||
|
|
||
| - name: Verify exact-head path policy and syntax | ||
| shell: bash --noprofile --norc -e -o pipefail {0} | ||
| run: | | ||
| test "$(git rev-parse HEAD)" = "${{ github.event.pull_request.head.sha || github.sha }}" | ||
| python -m coverage run -m pytest tests -q | ||
| bash scripts/ci/test_strix_quick_gate.sh | ||
| python -m compileall -q tests/test_strix_changed_path_policy.py tests/test_strix_workflow_dependency_hashes.py | ||
| bash -n scripts/ci/strix_quick_gate.sh | ||
| git diff --exit-code | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| # Strix legal Git path compatibility | ||
|
|
||
| ## Incident and buyer impact | ||
|
|
||
| The organization-required Strix quick gate rejected the exact changed-file list | ||
| for `ContextualWisdomLab/aFIPC#160` at head | ||
| `804ea97cd83144f94c5020a9d42f2573cc8cb442`. The pull request deletes generated | ||
| Packrat artifacts, including the tracked fixture path | ||
| `Ugly, but legal, path for a project (long)`. The central gate classified that | ||
| path as unsafe solely because its comma and parentheses were absent from the | ||
| bounded ASCII allowlist. Security analysis therefore stopped before examining | ||
| the pull request, leaving a valid supply-chain cleanup without exact-head Strix | ||
| evidence. | ||
|
|
||
| ## Decision | ||
|
|
||
| The normalizer now admits comma and ASCII parentheses. No other punctuation is | ||
| broadened. Existing fail-closed controls remain authoritative: | ||
|
|
||
| - empty, dot, absolute, leading/trailing-whitespace, NUL, CR, LF, and | ||
| backslash forms are rejected; | ||
| - raw `..` components are rejected before `posixpath.normpath()` can collapse | ||
| an embedded traversal such as `safe/../target.txt`; | ||
| - shell metacharacters such as semicolon, dollar sign, backtick, pipe, and | ||
| ampersand remain rejected; | ||
| - only the existing Unicode letter, combining-mark, and number categories are | ||
| accepted outside ASCII; | ||
| - `Path.resolve(strict=False)` followed by `relative_to()` proves containment | ||
| beneath the trusted repository root; and | ||
| - downstream Git and filesystem operations receive normalized paths as quoted | ||
| arguments, never as executable shell source. | ||
|
|
||
| This is a compatibility correction, not a general relaxation to every pathname | ||
| byte Git can represent. The privileged scanner intentionally retains a smaller, | ||
| audited path policy. | ||
|
|
||
| ## Test-first evidence | ||
|
|
||
| `tests/test_strix_changed_path_policy.py` extracts and executes the exact Python | ||
| normalizer embedded in `scripts/ci/strix_quick_gate.sh`. The materializer first | ||
| requires the historical Packrat fixture regression to fail on protected main, | ||
| then applies the narrow allowlist change and requires the same test to pass. | ||
| A test-only exact-head commit first demonstrated that `safe/../target.txt` | ||
| passed after normalization; the production repair now rejects its raw `..` | ||
| component before normalization. Permanent tests preserve established punctuation | ||
| and reject traversal, absolute paths, controls, whitespace ambiguity, backslashes, | ||
| and representative shell punctuation. The dedicated workflow runs the complete | ||
| repository test suite through coverage.py and pytest whenever code or either | ||
| authoritative contract document changes. | ||
|
seonghobae marked this conversation as resolved.
|
||
|
|
||
| The complete shell regression is also part of the permanent quality job. | ||
| `scripts/ci/test_strix_quick_gate.sh` exercises the executable shell boundary, | ||
| and the workflow's path filter includes that script so a change cannot bypass | ||
| the suite merely because pytest does not collect shell files. | ||
|
|
||
| ## Workflow dependency integrity | ||
|
|
||
| The exact-head policy workflow downloads its Python test runner from PyPI, so | ||
| version pins alone are insufficient: a compromised index response or replaced | ||
| artifact could otherwise change executable CI code without a repository diff. | ||
| The workflow therefore uses pip hash-checking mode (`--require-hashes`) together | ||
| with `--only-binary=:all:` and the exact SHA-256 digest of every wheel selected | ||
| on the fixed `ubuntu-24.04` x86-64 / CPython 3.14 runner: | ||
|
|
||
| - coverage 7.15.2: `b9a6367e4aff723e8ee8190836836124284e8fcd4265e307c844010cfa074f3f`; | ||
| - iniconfig 2.1.0: `9deba5723312380e77435581c6bf4935c94cbfab9b1ed33ef8d238ea168eb760`; | ||
| - packaging 26.2: `5fc45236b9446107ff2415ce77c807cee2862cb6fac22b8a73826d0693b0980e`; | ||
| - pluggy 1.6.0: `e920276dd6813095e9377c0bc5566d94c932c33b27a3e3945d8389c374dd4746`; | ||
| - Pygments 2.20.0: `81a9e26dd42fd28a23a2d169d86d7ac03b46e2f8b59ed4698fb4785f946d0176`; | ||
| - pytest 9.1.1: `37a86b45efb9a47a61a36449063e8e18d0cab3161329fc099eb21783169c4f0c`. | ||
|
|
||
| `tests/test_strix_workflow_dependency_hashes.py` was committed before the | ||
| workflow implementation and fails against the preceding exact head because | ||
| hash-checking mode and its trigger path are absent. It is now part of the | ||
| workflow's own path filter and verifies every requirement/digest pair. Any | ||
| package or runner-platform change must update the package version, PyPI wheel | ||
| digest, regression contract, and this record together. A digest mismatch must | ||
| fail closed; do not disable hash checking to restore availability. | ||
|
|
||
| ## Trusted workflow-source boundary | ||
|
|
||
| A review suggestion proposed `workflow_dispatch` so operators could rerun this | ||
| quality job manually. That isolated suggestion conflicts with the stronger | ||
| central automation boundary: GitHub's manual workflow UI and API allow the | ||
| caller to select a branch or tag, and the selected revision supplies the | ||
| workflow definition before any in-workflow checkout or source validation can | ||
| run. A credentialed central workflow must therefore not expose branch-selected | ||
| manual execution unless a separate protected-default-branch dispatcher first | ||
| validates immutable target metadata. | ||
|
|
||
| The conflict was captured rather than silently ignored. Exact-head run | ||
| `31156812291`, job `92798043647`, executed the complete central suite after | ||
| `workflow_dispatch` was added and failed the organization contract | ||
| `test_no_central_workflow_exposes_branch_selected_manual_dispatch` with | ||
| `1 failed, 969 passed`. The workflow now remains pull-request-triggered only, | ||
| and `tests/test_strix_workflow_dependency_hashes.py` permanently rejects | ||
| reintroduction of branch-selected manual source. The valid parts of the review | ||
| remain implemented: the shell regression is a trigger path and is executed by | ||
| the permanent exact-head job. | ||
|
|
||
| A future operator/API rerun must be a separately designed default-branch-only | ||
| entrypoint that treats target repository, pull-request number, and exact head | ||
| SHA as untrusted bounded data. It must never execute a caller-selected workflow | ||
| revision or receive broader credentials merely to improve convenience. | ||
|
|
||
| ## Rollback and incident response | ||
|
|
||
| Roll back the allowlist and regression together only if a downstream call is | ||
| proven to evaluate normalized paths as shell source. Until that defect is fixed, | ||
| fail Strix closed and retain the offending path, workflow run, and commit SHA as | ||
| incident evidence. Do not bypass the required security check. | ||
|
|
||
| If an exact dependency wheel becomes unavailable, first verify the release and | ||
| artifact digest against PyPI's file record and provenance. A rollback may select | ||
| the last known-good fully versioned wheel only when its exact hash is recorded in | ||
| the workflow, regression contract, and this document. Never replace | ||
| `--require-hashes` with an unhashed install. | ||
|
|
||
| Do not restore `workflow_dispatch` to this executable central workflow as an | ||
| availability workaround. Use a new pull-request event, a protected-main change, | ||
| or a separately reviewed immutable-target dispatcher. | ||
|
|
||
| ## References | ||
|
|
||
| Batchelder, N., & contributors. (2026). *coverage.py 7.15.2* [Computer | ||
| software]. Python Package Index. https://pypi.org/project/coverage/7.15.2/ | ||
|
|
||
| GitHub. (2026). *Manually running a workflow*. GitHub Docs. | ||
| https://docs.github.com/en/actions/how-tos/manage-workflow-runs/manually-run-a-workflow | ||
|
|
||
| GitHub. (2026). *Events that trigger workflows*. GitHub Docs. | ||
| https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows | ||
|
|
||
| Git Project. (2026). *Git index format*. https://git-scm.com/docs/index-format | ||
|
|
||
| Git Project. (2026). *git-ls-tree documentation*. https://git-scm.com/docs/git-ls-tree | ||
|
|
||
| Python Packaging Authority. (2026). *Secure installs*. pip documentation. | ||
| https://pip.pypa.io/en/stable/topics/secure-installs/ | ||
|
|
||
| Python Software Foundation. (2026). *pathlib—Object-oriented filesystem paths | ||
| (Python 3.14.6 documentation)*. https://docs.python.org/3.14/library/pathlib.html | ||
|
|
||
| pytest development team. (2025). *iniconfig 2.1.0* [Computer software]. Python | ||
| Package Index. https://pypi.org/project/iniconfig/2.1.0/ | ||
|
|
||
| pytest development team. (2025). *pluggy 1.6.0* [Computer software]. Python | ||
| Package Index. https://pypi.org/project/pluggy/1.6.0/ | ||
|
|
||
| pytest development team. (2026). *pytest 9.1.1* [Computer software]. Python | ||
| Package Index. https://pypi.org/project/pytest/9.1.1/ | ||
|
|
||
| Python Packaging Authority. (2026). *packaging 26.2* [Computer software]. | ||
| Python Package Index. https://pypi.org/project/packaging/26.2/ | ||
|
|
||
| Pygments contributors. (2026). *Pygments 2.20.0* [Computer software]. Python | ||
| Package Index. https://pypi.org/project/Pygments/2.20.0/ | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| """Regression tests for the production Strix changed-path normalizer.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import subprocess | ||
| import sys | ||
| import tempfile | ||
| import unittest | ||
| from pathlib import Path | ||
|
|
||
|
|
||
| REPOSITORY_ROOT = Path(__file__).resolve().parents[1] | ||
| GATE_SCRIPT = REPOSITORY_ROOT / "scripts" / "ci" / "strix_quick_gate.sh" | ||
| START_MARKER = 'python3 - "$REPO_ROOT" "$changed_file" <<\'PY\'\n' | ||
| END_MARKER = "\nPY\n}\n\nnormalize_changed_files_cache()" | ||
| LEGAL_PACKRAT_PATH = ( | ||
| "packrat/lib/x86_64-pc-linux-gnu/3.4.1/packrat/tests/testthat/" | ||
| "Ugly, but legal, path for a project (long)/bread/DESCRIPTION" | ||
| ) | ||
|
|
||
|
|
||
| def _normalizer_source() -> str: | ||
| """Return the exact embedded Python program used in production.""" | ||
|
|
||
| gate_source = GATE_SCRIPT.read_text(encoding="utf-8") | ||
| prefix, separator, remainder = gate_source.partition(START_MARKER) | ||
| if not separator or not prefix: | ||
| raise AssertionError("Strix changed-path normalizer start marker is missing") | ||
| source, separator, _suffix = remainder.partition(END_MARKER) | ||
| if not separator: | ||
| raise AssertionError("Strix changed-path normalizer end marker is missing") | ||
| return source | ||
|
|
||
|
|
||
| def _normalize(candidate: str) -> subprocess.CompletedProcess[str]: | ||
| """Execute the production normalizer with an isolated repository root.""" | ||
|
|
||
| with tempfile.TemporaryDirectory() as temporary_directory: | ||
| return subprocess.run( | ||
| [sys.executable, "-c", _normalizer_source(), temporary_directory, candidate], | ||
| check=False, | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
|
|
||
|
|
||
| class StrixChangedPathPolicyTests(unittest.TestCase): | ||
| """Verify legal Git paths and fail-closed path boundaries.""" | ||
|
|
||
| def test_accepts_historical_packrat_fixture_path(self) -> None: | ||
| """A tracked Packrat fixture with commas and parentheses is valid input.""" | ||
|
|
||
| result = _normalize(LEGAL_PACKRAT_PATH) | ||
| self.assertEqual(result.returncode, 0, result.stderr) | ||
| self.assertEqual(result.stdout.strip(), LEGAL_PACKRAT_PATH) | ||
|
|
||
| def test_preserves_existing_supported_punctuation(self) -> None: | ||
| """Existing bracket, at-sign, plus-sign, space, and hyphen support remains.""" | ||
|
|
||
| candidate = "ui/[slug]/128x128@2x +page-safe/file-name.ts" | ||
| result = _normalize(candidate) | ||
| self.assertEqual(result.returncode, 0, result.stderr) | ||
| self.assertEqual(result.stdout.strip(), candidate) | ||
|
|
||
| def test_rejects_traversal_absolute_controls_and_shell_punctuation(self) -> None: | ||
| """The repair does not admit traversal, controls, or shell syntax.""" | ||
|
|
||
| rejected = ( | ||
| "", | ||
| ".", | ||
| "..", | ||
| "../secret.txt", | ||
| "safe/../target.txt", | ||
| "/tmp/secret.txt", | ||
| "safe\\escape.txt", | ||
| "safe\nname.txt", | ||
| "safe\rname.txt", | ||
| " leading.txt", | ||
| "trailing.txt ", | ||
| "safe;command.txt", | ||
| "safe$(command).txt", | ||
| "safe`command`.txt", | ||
| "safe|command.txt", | ||
| "safe&command.txt", | ||
| ) | ||
|
seonghobae marked this conversation as resolved.
|
||
| for candidate in rejected: | ||
| with self.subTest(candidate=repr(candidate)): | ||
| result = _normalize(candidate) | ||
| self.assertNotEqual(result.returncode, 0) | ||
| self.assertEqual(result.stdout, "") | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.