Repository navigation
fix(release): bump the version everywhere CI checks, not just pyproject - #8
Merged
Merged
Conversation
v1.0.2b7 shipped with pyproject.toml, uv.lock and CHANGELOG bumped, but the FnOS packaging manifests still said 1.0.2b6, so tests/unit/test_fnos_docker_persist.py:: test_fnos_manifest_version_matches_pyproject failed and the Release workflow's "Build distributions" job died at Run tests. Publish to PyPI, Create GitHub Release, Trigger FnOS FPK build and Trigger sync main into develop were all skipped, so nothing was published under the Octop name. main was left internally inconsistent: pyproject 1.0.2b7 against manifests 1.0.2b6, with the v1.0.2b7 tag sitting on it. Bumps the remaining five locations that .cursor/skills/publish/SKILL.md lists for a release: src/octop/__init__.py __version__, fnos/docker/manifest, fnos/native/manifest, the FnOS docker-compose image tag, and the shields.io version badges in README.md and README_CN.md. Why the pre-commit hook did not catch this: the release commit changed only pyproject.toml, uv.lock and CHANGELOG.md. pytest-testmon selects tests by import graph, and nothing imports pyproject.toml, so the affected set was empty and the gate ran zero tests. That is the documented trade-off in the Makefile - "this is local feedback only, so a missed cross-module impact is caught there" - and here CI is what caught it. Worth remembering that a data-only change to a manifest or lockfile is invisible to testmon by construction. Verification: make all = 4624 passed, 25 skipped, 0 failed, exit 0. tests/unit/test_fnos_docker_persist.py and test_self_update.py: 165 passed.
… netloc SEC-2, Windows only. Caught by Windows CI, not by anything local. `file_url_to_abs_path` read only `urlparse(...).path`. For the canonical three-slash form `file:///C:/…` that is correct — the drive is in the path. But Windows CI builds these URLs from `Path.as_posix()`, which produces the two-slash form `file://C:/…`. There `urlparse` puts `C:` in **netloc** and the path is just `/Users/…`, so the drive was silently dropped. The result is a drive-relative path. On the GitHub Actions Windows runner the workdir is on D:, so `/Users/runneradmin/.octop/…` resolved to `D:/Users/runneradmin/.octop/…` rather than the file the caller named. That breaks the SEC-2 guarantee in two ways at once: - `test_file_url_read_outside_allowlist_forbidden` read D:/… , found nothing, and returned 404 instead of 403. - `test_file_url_write_outside_allowlist_forbidden` **wrote the file and returned 200**. The containment check ran against the mangled path, which was inside an allowed root, so it passed. A user-controlled `path` query parameter could therefore cause a write outside the intended absolute location on Windows, with no 403. The fix reads netloc and, when it is a bare drive letter, prepends it to the path. A UNC host (`file://server/share/…`) is not a drive letter and is left to the existing branch. This is a defect in our own SEC-2 work (d08a692), not an upstream regression. Regression test is platform-independent so it runs on Linux too — it asserts the URL shape, not the filesystem. Verified it fails on the old code (1 failed, 12 passed) and passes on the new (13 passed). make all: 4625 passed, 25 skipped, 0 failed, exit 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Hotfix for
v1.0.2b7, cut frommainper AGENTS.md section 10.The problem
v1.0.2b7bumpedpyproject.toml,uv.lockandCHANGELOG.md, but not the FnOS packaging manifests, which still said1.0.2b6. So:tests/unit/test_fnos_docker_persist.py::test_fnos_manifest_version_matches_pyprojectfailedBuild distributionsjob died atRun tests(exit 2)Publish to PyPI,Create GitHub Release,Trigger FnOS FPK buildandTrigger sync main into developwere all skippedmainwas internally inconsistent -pyproject.tomlat 1.0.2b7 against manifests at 1.0.2b6 - with thev1.0.2b7tag sitting on it.What this changes
The five locations
.cursor/skills/publish/SKILL.mdlists for a release, beyond the three already done:src/octop/__init__.py-__version__fnos/docker/manifest-version=fnos/native/manifest-version=fnos/docker/app/docker/docker-compose.yaml- image tagREADME.md/README_CN.md- shields.io version badgesWhy the pre-commit hook missed it
The release commit changed only
pyproject.toml,uv.lockandCHANGELOG.md. pytest-testmon selects tests by import graph and nothing importspyproject.toml, so the affected set was empty and the gate ran zero tests.That is the documented trade-off in the Makefile - "this is local feedback only, so a missed cross-module impact is caught there". Here CI is what caught it. Worth recording: a data-only change to a manifest or lockfile is invisible to testmon by construction, so those files need manual attention on every release.
Verification
make all- 4624 passed, 25 skipped, 0 failed, exit 0tests/unit/test_fnos_docker_persist.py+test_self_update.py- 165 passedNote on the tag
v1.0.2b7is left where it is, pointing at the commit with the old manifests. Future fork releases should carry a fork-local marker such as+ra.1so the next upstream merge cannot produce two different releases claiming 1.0.2b7.Second finding: a Windows-only hole in our own SEC-2 work
Windows CI caught a real defect, so this PR carries two fixes.
file_url_to_abs_pathread onlyurlparse(...).path. That is correct for the canonicalfile:///C:/…, but Windows CI builds these URLs fromPath.as_posix(), which yields the two-slashfile://C:/…— whereurlparseputsC:in netloc and the path is only/Users/…. The drive was dropped, yielding a drive-relative path (D:/Users/...on the runner).That broke the SEC-2 guarantee twice:
A user-controlled
pathquery parameter could cause a write outside the intended absolute location on Windows, with no 403. This is a defect in our SEC-2 work (d08a6928), not an upstream regression.Fix reads netloc and, when it is a bare drive letter, prepends it. A UNC host (
file://server/share/...) is not a drive letter and takes the existing branch.The regression test asserts the URL shape, not the filesystem, so it runs on Linux too. Confirmed red on the old code (1 failed, 12 passed) and green on the new (13 passed).
Verification
make all— 4625 passed, 25 skipped, 0 failed, exit 0