Repository navigation
docs(security): add the rows the October review held back for GHSA-rxjw-584q-hxjr - #1650
Merged
Merged
Conversation
…jw-584q-hxjr The advisory was published on 31 October 2026 with 3.4.0, so the two rows the October 2026 review held back under coordinated disclosure are filled in: the boundary 1 row on a request path, which did not hold until 3.4.0, and finding 8 with its advisory, the releases it affects and the commit that fixed it. Signed-off-by: José M. Requena Plens <mail@jmrp.io>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Document finding 8 and path protections docs/development/security-review-2026-10.md |
The review record adds path-boundary details and replaces the coordinated-disclosure placeholder with finding 8’s affected versions, exposure conditions, causes, fix, and advisory publication date. |
Estimated code review effort: 1 (Trivial) | ~5 minutes
Change: Other
Possibly related issues
- Record a security review, and send CSP and X-Frame-Options from the documentation site #1543: This change updates the security review record requested by the issue.
- OpenSSF Best Practices: reach passing and silver, and record what gold needs #1536: This change adds security review evidence for the OpenSSF
security_reviewcriterion.
Possibly related PRs
- jmrplens/gitlab-mcp-server#1564: Both changes update the October security review record and address the finding 8 disclosure timeline.
Suggested labels: documentation, security, v3.4.0
Suggested reviewers: sourcery-ai
Merge Risk: 
340f2
-
The update records finding 8 and its path protections without changing runtime behavior. No actionable merge-blocking issue is identified.
Pre-merge checks: 
-
Check name Status Explanation Resolution Description check The description clearly explains the documentation changes and lists validation commands, but it omits the required template sections, including Related Issue, Type of Change, Changes Made, How to Tes… Rewrite the description using the repository template. Add a related issue or advisory reference, select Documentation update, list the changes, provide numbered test steps, state N/A for breaking changes if applicable, and complete the app… -
Check name Status Explanation Title check The title clearly identifies a documentation security change that adds the withheld October review rows for GHSA-rxjw-584q-hxjr. Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
-
Full details: Description check
-
Explanation
The description clearly explains the documentation changes and lists validation commands, but it omits the required template sections, including Related Issue, Type of Change, Changes Made, How to Test, Breaking Changes / Migration Notes, and the Checklist.
Resolution
Rewrite the description using the repository template. Add a related issue or advisory reference, select Documentation update, list the changes, provide numbered test steps, state N/A for breaking changes if applicable, and complete the applicable checklist items. Preserve the existing security-review summary and validation commands.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
github-actions
Bot
added
documentation
|
This was referenced Oct 31, 2026
Merged
jmrplens
added a commit
that referenced
this pull request
Oct 31, 2026
…ash keeps its sign-offs (#1651) SECURITY.md said one advisory had been published and that an advisory follows its release by a few days. Neither has held since 31 October, when GHSA-rxjw-584q-hxjr was published the day 3.4.0 was released. The page now lists both advisories with their severity and dates, describes the second way a fix is developed (in the advisory's temporary private fork, merged from the advisory, where GitHub runs no CI), and says an advisory is published once the release that carries the fix is, which can be the same day. That is the published process of `vulnerability_report_process` and the silver `vulnerability_response_process`. It also names two kinds of vulnerability that finding 8 belonged to and the page did not list. One is a tool argument that makes a request name another GitLab endpoint. The other is a call that gets past one of the per-action controls (the tool exclusion list, the tier filter, the chosen surface, a destructive action's confirmation), which are enforcement like the read-only and safe modes. Input Validation says what 3.4.0 does with an argument placed into a request path. CONTRIBUTING.md and the About page, in English and Spanish, now name the email channel beside GitHub's private reporting, with the OpenPGP key, as SECURITY.md already did, for `vulnerability_report_private`. CONTRIBUTING.md said GitHub copies the pull request's sign-offs into the squash commit. With `squash_merge_commit_message` set to `PR_BODY` that holds only for GitHub's default message: the squash commits of #1584, #1585, #1593 and #1646 to #1650 carry no `Signed-off-by`, while every commit of those pull requests does, as the API shows. The page now says the DCO job judges the pull request's commits rather than the squash, and that a message written by hand at the merge should end with their sign-offs. `cmd/audit_dco/doc.go`, its reference in `docs/development/cmd-utilities.md` and the comment above the `dco` job in `ci.yml` made the same claim and are corrected, and they and GOVERNANCE.md name the advisory fork merge as a way to `main` that nothing in CI judges. This serves the silver `dco`. GOVERNANCE.md, the document the silver `governance` asks for, counted two deploy keys with write access. The repository has one, the key the release workflow pushes the version stamp with, and the page now says so. Each of these corrects a page that had fallen behind the project, which is what `documentation_current` asks. I checked the dates on 31 October through the advisories and releases APIs: GHSA-rxjw-584q-hxjr was published 27.5 minutes after the v3.4.0 release, and neither advisory has a CVE yet. Of the 50 commits on `main` since 28 October besides the release stamp, only those 8 squashes lack a `Signed-off-by`. The page does not claim 3.4.0 reached every channel before its advisory: it never reached winget, whose submission failed in the release run under `continue-on-error`. The About page's FAQ answer and the issue templates still name only GitHub's private reporting. Signed-off-by: José M. Requena Plens <mail@jmrp.io> --------- Signed-off-by: José M. Requena Plens <mail@jmrp.io>
jmrplens
added a commit
that referenced
this pull request
Oct 31, 2026
… included (#1655) The October security review record still read as if finding 8 were held back, and a review of the rows #1650 added found places where they said more, or less, than the code and the advisory do. This corrects them and closes the sign-off, for the OpenSSF Best Practices gold criterion `security_review`, which asks for a security review a person performed as well as tools. The boundary 1 path row said `EscapePathSegment` and `EscapePathSegments` escape every argument it lists. The model registry download uses neither: its arguments are escaped with `url.PathEscape`, by client-go's own route under a directory and by `rootFilePath` at the package root. The row now says so, and says what its model registry test holds. The record said every requirement was traced at `8e959c3d8`, but the path row describes code that arrived with `39b7093af`, the commit 3.4.0 was released from. The introduction, the scope, the method and the Sign-off table now name that commit for the path row. Finding 8 now names the harm its advisory gives: the request ran under the operator's token, or the caller's in HTTP mode, past the server's per-action controls. It states which GitLab releases refuse the path and with which flags, and its status records the release, that the fix was merged from the advisory's private fork, and that the advisory is not yet in the GitHub Advisory Database or OSV and its CVE is unassigned. Findings 3, 4 and 5 still waited for a release that has shipped. Each keeps the status signed off on 27 October and gains a dated update: #1560 corrected the policy, and #1554 and Go 1.27.3 shipped in 3.4.0, before the 2 November deadline finding 4 named. The dependencies row says the same. I read the boundary 1 path row and finding 8 on 31 October and sign the review as of that date, every finding included. The record now answers the question it left open on finding 8: of the rows recorded on 27 October it moves only input validation, whose untraced "String parameters are sanitized where applicable" did not hold for a path argument until 3.4.0, and the others keep their results, since every request stayed on the instance under the token's own authority, read-only mode removed every write it reaches, safe mode previewed each, and a read stayed a GET. Two dated updates follow from the layers below: - The input validation row now names the stdio test that holds the SDK's validation on the individual surface, which the record had left untraced. - Finding 8 records my decision of 31 October: SECURITY.md now names the request-path traversal as a kind of vulnerability, and the per-action controls as a requirement. SECURITY.md's advisory count and disclosure steps, and the assurance case, are corrected for finding 8 in the first two layers of this stack. Signed-off-by: José M. Requena Plens <mail@jmrp.io> --------- Signed-off-by: José M. Requena Plens <mail@jmrp.io>
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.
GHSA-rxjw-584q-hxjr was published on 31 October 2026, once 3.4.0 was live on every channel, so this fills in the two rows the October 2026 security review held back under coordinated disclosure.
The boundary 1 table gains the path row, "An argument cannot redirect a request to an unintended path", recorded as not holding until 3.4.0, with what enforces it now and the tests that hold it.
Finding 8 gets its description, the releases it affects, its severity and the commit that fixed it, and the introduction says when and why the two rows were added.
I ran
format_md_tables, markdownlint,check-doc-linksandcheck-em-dash.