chore:ci security updates - #2
Open
NivGreenstein wants to merge 4 commits into
Open
Conversation
CONTRIBUTING.md § Conventions specifies `var ErrErrorName = errors.New(...)`, and the repo extends the Err* prefix to error types: 82 Err*-prefixed types elsewhere, and zero exported *Error-suffixed ones. tms inverted that for its eight struct types while its own sentinel vars (ErrNoTransformBackend, ErrTMSVersion1, ErrGridNotActivated) already followed it, so the package was inconsistent with the repo and with itself. server/ogc, added in the same work, follows the convention — this brings tms into line with both. ErrVariableWidthUnsupported and ErrGridUnavailable also move out of registry.go into errors.go, where every other package keeps its errors (cache/errors.go, config/errors.go, provider/errors.go). No behaviour changes: the renames are mechanical, every usage was already confined to tms/, and no error string is touched. Kept as its own commit for that reason — it touches many lines and none of them do anything. Addresses S1 and S2 of review.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six findings from the two-axis review, none of which changed a documented behaviour except where the review asked for it. P10 — cache.NewKey nil-dereferenced on a nil grid. Its own comment says a grid is required and deliberately not defaulted, then called grid.ID() with no guard. It now returns (Key, error) with cache.ErrNilGrid; all four call sites already had an error path. Declined the alternative of making TileMatrixSet.ID() nil-safe: an empty id reads as WebMercatorQuad in Key.String, so that would have turned a panic into the exact silent misfiling the comment forbids — another scheme's tiles under WebMercatorQuad's keys, visible only as a cache that never hits. P2 — /api answered 200 to any ?f=, alone on the surface. It now runs through negotiate, so ?f=html and ?f=pbf are 400 like everywhere else, and openapi.json declares the parameter it honours. The response keeps the OpenAPI media type rather than the negotiated format's: "json" names the representation, and for this resource that representation is an OpenAPI 3.0 definition, which OGC requires carry the specific profile. S3 — the "MatrixSize then bound x and y independently" shape existed in four places, spelled two different ways (inclusive max in two, exclusive count in the others). Per-axis validation is what makes the non-square grids safe, so four copies was four chances to drift. Extracted as TileMatrixSet.ValidateTile, which returns a structured ErrTileOutsideMatrix carrying the axis and the matrix, so each caller still phrases the failure in its own vocabulary — X/Y natively, tileCol/tileRow on the OGC routes — without repeating the arithmetic. Every existing error string is preserved. Two message changes fall out of it, neither covered by a test: a non-numeric tileRow/tileCol no longer carries the "; tile matrix N has M rows" suffix, since it is a parse failure rather than a range one, and when both row and column are out of range the message now names tileCol where it named tileRow. S4 — the per-request grid was smuggled through a mutated Map copy at two independent sites. Both now call atlas.Map.InGrid, which names the intent in one place. Stopped short of the review's suggestion that Encode take the grid as a parameter: the pinned Map also feeds cache.NewKey inside SeedMapTile, so removing the carrier means changing Map.Encode, Atlas.SeedMapTile and Atlas.PurgeMapTile — three exported signatures, and a call worth making deliberately rather than as a side effect of this. S5 — the tileset link base was assembled in four places, three of them identical. Extracted as tileSetPath. ValidateTile's test is deliberately not in tilematrixset_test.go: that file is the morecantile port's correctness oracle and every value in it is upstream's. Verified in both CGO modes as CONTRIBUTING.md requires, with gofmt -s clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Update vulnerable dependencies and make the local CI workflow resilient to missing credentials and occupied ports. Replace brittle MVT byte-size checks with decoded tile assertions.
Coverage Report for CI Build 87144Warning No base build found for commit Coverage: 47.647%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
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
2 participants
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.
No description provided.