From 8b67c56123c3e9c7e309e2e598d8983fe5097114 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Sat, 31 Oct 2026 08:40:08 +0100 Subject: [PATCH 1/2] docs(security): correct the October review's path row and bring its findings to 3.4.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The boundary 1 path row said EscapePathSegment and EscapePathSegments escape every argument it lists, the model registry download's included. That handler 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 names the model registry test for what it holds. The record said every requirement was traced at 8e959c3d8, where the path row describes code that arrived with 39b7093af, the commit 3.4.0 was released from. The introduction, the scope and the method now name that commit for the path row. Finding 8 now says the request ran under the operator's token, or the caller's in HTTP mode, past the server's per-action controls, which is the harm its advisory names; 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, with its CVE unassigned. Findings 3, 4 and 5 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, #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. Signed-off-by: José M. Requena Plens --- docs/development/security-review-2026-10.md | 49 ++++++++++++--------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/docs/development/security-review-2026-10.md b/docs/development/security-review-2026-10.md index 49bce206d..4b1d7e7eb 100644 --- a/docs/development/security-review-2026-10.md +++ b/docs/development/security-review-2026-10.md @@ -12,7 +12,13 @@ maintainer read the record against the code at that commit and signed it off on 27 October 2026, as [Sign-off](#sign-off) records, with finding 8 held back under coordinated disclosure until its advisory is published. The advisory, GHSA-rxjw-584q-hxjr, was published on 31 October 2026 with 3.4.0, and the two -rows it held back, the boundary 1 path row and finding 8, were added then. +rows it held back, the boundary 1 path row and finding 8, were added then. The +path row's Enforced by and Held by columns describe the code at +[`39b7093af`](https://github.com/jmrplens/gitlab-mcp-server/commit/39b7093afffede232184c1e385b1ef4a215b0258), +the commit 3.4.0 was released from, since the requirement did not hold at the +commit reviewed, and finding 8 was checked against the releases its advisory +names. The statuses of findings 3, 4 and 5 were brought up to date the same +day, each in a dated update after the status signed off on 27 October. --- @@ -33,7 +39,8 @@ finding is tracked. ## Scope -- **Reviewed:** the server, `cmd/server` and `internal/`, at the commit above; +- **Reviewed:** the server, `cmd/server` and `internal/`, at the commit above, + and for the boundary 1 path row at `39b7093af`; its release configuration (`.goreleaser.yml`, `.github/workflows/release.yml` and the `Dockerfile`) and what the 3.3.0 release published from it; the repository's code scanning, Dependabot and secret scanning alerts; and the @@ -56,7 +63,9 @@ finding is tracked. ## Method **Tracing.** Each requirement was traced to the code that enforces it, read at -the commit, and to the tests or gates that would fail if it stopped holding. A +the commit (the boundary 1 path row excepted, which was traced at `39b7093af`, +as the introduction says), and to the tests or gates that would fail if it +stopped holding. A requirement is recorded as holding only when both exist, and a statement this preparation found no code or no test for is recorded as not traced. @@ -95,12 +104,12 @@ was loaded into every one of the site's 216 pages in Chromium (see ### 1. Model to server -| Requirement | Enforced by | Held by | Result | -| ----------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------- | -| An argument is not authority to exceed the token's own permissions | Every request carries the operator's token, or in HTTP mode the caller's, and GitLab authorizes it; the server holds no GitLab credential of its own that could widen it, and narrows what it serves a `read_api` or fine-grained token to what GitLab accepts from it ([ADR-0018](adr/adr-0018-authorization-admits-per-action-gating.md), [ADR-0024](adr/adr-0024-fine-grained-token-authority-per-action.md), [ADR-0026](adr/adr-0026-read-api-token-served-what-gitlab-accepts.md)) | GitLab's authorization of each request, which nothing here bypasses; the narrowing is held by `test/e2e/http/fine_grained_test.go` and `test/e2e/stdio/token_scope_test.go`, which drive the real binary | Holds | -| `GITLAB_MCP_READ_ONLY` and `GITLAB_MCP_SAFE_MODE` hold on every surface | `internal/tools/catalog_filter.go` removes, or turns into a preview, every mutating action before any surface is built from the catalog, and dynamic execute answers a withheld action as withheld | `TestCreateServer_ReadOnly_*` and `TestCreateServer_SafeMode_*` (`cmd/server`) on the three surfaces; `TestFilterActionCatalog_*` (`internal/tools`); `make check-readonly-graphql`, no read-only action can reach a GraphQL mutation; gate 5 of `make check-action-grants-derivation`, an action is read-only exactly when a `read_api` token reaches what it sends, with each departure declared | Holds | -| An argument cannot send a request to another host | client-go builds every request against the client's base URL and refuses another scheme or host; in HTTP mode the `GITLAB-URL` header only picks among the instances `--gitlab-url` publishes (#479, #645); a redirect off the instance, or down to http, loses every credential header (`credentialSafeRedirect`, `internal/gitlab/redirect.go`) | `TestGate_PublishedInstances_HeaderCannotRedirectTheCredential`, `TestGate_NoPublishedInstanceAndNoHatch_RefusesToStart` and `TestGate_TheEscapeHatchIsRefusedOnAReachableListener` (`test/e2e/http/gate_test.go`); `TestRedirect_OffTheInstanceCarriesNoCredentialAndIsRecorded` (`test/e2e/http/redirect_test.go`) | Holds | -| An argument cannot redirect a request to an unintended path | A few client-go routes interpolate a path argument without escaping it (`NoEscape`), the model registry route escapes its arguments but leaves their dots, and `dotUnescapeTransport` turns the `%2E` of any escaped argument back into `.`, so a value that is `.` or `..` as a whole is a dot segment there too. `pathTraversalGuardTransport` (`internal/gitlab/path_guard.go`), just inside `dotUnescapeTransport` in the transport chain every surface and handler reaches GitLab through, refuses on every hop any request whose path has a `.` or `..` segment, as given or once percent-decoded (`gitlab.HasDotSegment`, the one statement of the rule), with `gitlab.ErrRequestPathRefused`, which `toolutil.ClassifyError` and the `WrapErr` family report as a refused path argument ahead of the network and TLS branches. `toolutil.RejectTraversalArg` applies the same rule before the request is built, naming the parameter, in every handler of those routes: jobs (`artifact_path`, `ref_name`), system hooks and group hooks (`key`, `trigger`), pipeline schedule variables (`key`), license and project templates (`key`, `template_type`), and the model registry download (`model_version_id`, `filename`, and through `rejectUnresolvedDotSegment` each `/`-separated component of `path` that `path.Clean` keeps as a name, so a dot segment marked off by a backslash or written percent-encoded is refused while a `.` or `..` between slashes is still resolved). `toolutil.EscapePathSegment` and `toolutil.EscapePathSegments` then escape each such argument as one segment, or per segment for an artifact path and the archive ref | `TestHasDotSegment_PathForms_ReportOnlyDotSegments`, `TestPathTraversalGuardTransport_RoundTrip_RefusesDotSegmentsAndSendsCleanPaths` and `TestAPITransport_UnescapedDotSegmentArgument_IsRefusedBeforeSending` (`internal/gitlab/path_guard_test.go`); `TestClassifyError_RequestPathRefused_SaysThePathArgumentWasRefused`, `TestWrapErr_RequestPathRefused_ReplacesTheHandlersHint` and `TestWrapErrWithMessage_PathRefusedByTheClient_IsARefusedArgument` (`internal/toolutil/errors_test.go`); `TestRejectTraversalArg_*`, `TestEscapePathSegment_Values_AreEscapedAsOneSegment` and `TestEscapePathSegments_Values_AreEscapedPerSegment` (`internal/toolutil/path_args_test.go`); in each handler package a `*_DotSegment*_IsRefused` test (no request reaches the mock) and an exact-path `*_IsEscapedAsOneSegment` or `*_IsEscapedPerSegment` test: `internal/tools/jobs`, `systemhooks`, `groups` (`TestHookSubOperations_PathArgument_IsEscapedAsOneSegment`), `pipelineschedules`, `licensetemplates`, `projecttemplates`, and `modelregistry` (`TestDownload_DotSegmentVersionOrFilename_IsRefused`, `TestDownload_DotSegmentPath_IsRefusedWhereNormalizationKeepsIt`, `TestDownload_FileName_IsEscapedOnBothRoutes`) | Did not hold until 3.4.0, finding 8 | +| Requirement | Enforced by | Held by | Result | +| ----------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------- | +| An argument is not authority to exceed the token's own permissions | Every request carries the operator's token, or in HTTP mode the caller's, and GitLab authorizes it; the server holds no GitLab credential of its own that could widen it, and narrows what it serves a `read_api` or fine-grained token to what GitLab accepts from it ([ADR-0018](adr/adr-0018-authorization-admits-per-action-gating.md), [ADR-0024](adr/adr-0024-fine-grained-token-authority-per-action.md), [ADR-0026](adr/adr-0026-read-api-token-served-what-gitlab-accepts.md)) | GitLab's authorization of each request, which nothing here bypasses; the narrowing is held by `test/e2e/http/fine_grained_test.go` and `test/e2e/stdio/token_scope_test.go`, which drive the real binary | Holds | +| `GITLAB_MCP_READ_ONLY` and `GITLAB_MCP_SAFE_MODE` hold on every surface | `internal/tools/catalog_filter.go` removes, or turns into a preview, every mutating action before any surface is built from the catalog, and dynamic execute answers a withheld action as withheld | `TestCreateServer_ReadOnly_*` and `TestCreateServer_SafeMode_*` (`cmd/server`) on the three surfaces; `TestFilterActionCatalog_*` (`internal/tools`); `make check-readonly-graphql`, no read-only action can reach a GraphQL mutation; gate 5 of `make check-action-grants-derivation`, an action is read-only exactly when a `read_api` token reaches what it sends, with each departure declared | Holds | +| An argument cannot send a request to another host | client-go builds every request against the client's base URL and refuses another scheme or host; in HTTP mode the `GITLAB-URL` header only picks among the instances `--gitlab-url` publishes (#479, #645); a redirect off the instance, or down to http, loses every credential header (`credentialSafeRedirect`, `internal/gitlab/redirect.go`) | `TestGate_PublishedInstances_HeaderCannotRedirectTheCredential`, `TestGate_NoPublishedInstanceAndNoHatch_RefusesToStart` and `TestGate_TheEscapeHatchIsRefusedOnAReachableListener` (`test/e2e/http/gate_test.go`); `TestRedirect_OffTheInstanceCarriesNoCredentialAndIsRecorded` (`test/e2e/http/redirect_test.go`) | Holds | +| An argument cannot redirect a request to an unintended path | A few client-go routes interpolate a path argument without escaping it (`NoEscape`), the model registry route escapes its arguments but leaves their dots, and `dotUnescapeTransport` turns the `%2E` of any escaped argument back into `.`, so a value that is `.` or `..` as a whole is a dot segment there too. `pathTraversalGuardTransport` (`internal/gitlab/path_guard.go`), just inside `dotUnescapeTransport` in the transport chain every surface and handler reaches GitLab through, refuses on every hop any request whose path has a `.` or `..` segment, as given or once percent-decoded (`gitlab.HasDotSegment`, the one statement of the rule), with `gitlab.ErrRequestPathRefused`, which `toolutil.ClassifyError` and the `WrapErr` family report as a refused path argument ahead of the network and TLS branches. `toolutil.RejectTraversalArg` applies the same rule before the request is built, naming the parameter, in every handler of those routes: jobs (`artifact_path`, `ref_name`), system hooks and group hooks (`key`, `trigger`), pipeline schedule variables (`key`), license and project templates (`key`, `template_type`), and the model registry download (`model_version_id`, `filename`, and through `rejectUnresolvedDotSegment` each `/`-separated component of `path` that `path.Clean` keeps as a name, so a dot segment marked off by a backslash or written percent-encoded is refused while a `.` or `..` between slashes is still resolved). In every handler but the model registry download, `toolutil.EscapePathSegment` and `toolutil.EscapePathSegments` then escape each such argument as one segment, or per segment for an artifact path and the archive ref; the model registry download's arguments are escaped with `url.PathEscape`, by client-go's own route under a directory and by `rootFilePath` at the package root | `TestHasDotSegment_PathForms_ReportOnlyDotSegments`, `TestPathTraversalGuardTransport_RoundTrip_RefusesDotSegmentsAndSendsCleanPaths` and `TestAPITransport_UnescapedDotSegmentArgument_IsRefusedBeforeSending` (`internal/gitlab/path_guard_test.go`); `TestClassifyError_RequestPathRefused_SaysThePathArgumentWasRefused`, `TestWrapErr_RequestPathRefused_ReplacesTheHandlersHint` and `TestWrapErrWithMessage_PathRefusedByTheClient_IsARefusedArgument` (`internal/toolutil/errors_test.go`); `TestRejectTraversalArg_*`, `TestEscapePathSegment_Values_AreEscapedAsOneSegment` and `TestEscapePathSegments_Values_AreEscapedPerSegment` (`internal/toolutil/path_args_test.go`); in each handler package a `*_DotSegment*_IsRefused` test (no request reaches the mock) and an exact-path `*_IsEscapedAsOneSegment` or `*_IsEscapedPerSegment` test: `internal/tools/jobs`, `systemhooks`, `groups` (`TestHookSubOperations_PathArgument_IsEscapedAsOneSegment`), `pipelineschedules`, `licensetemplates`, `projecttemplates`, and `modelregistry` (`TestDownload_DotSegmentVersionOrFilename_IsRefused` and `TestDownload_DotSegmentPath_IsRefusedWhereNormalizationKeepsIt`, with `TestDownload_FileName_IsEscapedOnBothRoutes` in place of the exact-path test, holding the `url.PathEscape` escaping of both routes) | Did not hold until 3.4.0, finding 8 | ### 2. GitLab to server to model @@ -151,7 +160,7 @@ that no row rests on more is the maintainer's. | TLS: verification on by default, `GITLAB_MCP_SKIP_TLS_VERIFY` only where trusted | The GitLab client verifies certificates unless the skip is set, and it, the HTTPS listener and the health probe set TLS 1.2 as their minimum (`internal/gitlab/client.go`, `cmd/server/tls_reload.go`, `cmd/server/probe.go`); OAuth mode refuses the skip for an instance off loopback | `TestBuildDestinationPools_Verified_OnePairSharedAndLent` and `TestBuildDestinationPools_SkipTLSVerify_APairOfTheClientsOwn` (`internal/gitlab`); `TestTLSConfigFor_StatesTheFloorAndReloads`, `TestProbeTLSConfig_HoldsTheServersFloor` and `TestValidateHTTPAuthConfig_OAuthRefusesSkipTLSVerify` (`cmd/server`); `TestListener_TLS_RefusesObsoleteVersions` and `TestOAuth_SkipTLSVerifyScope` (`test/e2e/http`) | Holds | | Input validation | On the individual surface the SDK validates each call against the action's input schema before a handler runs. On the dynamic and meta surfaces it validates the envelope, and `toolutil.UnmarshalParams` decodes `params` into the action's typed input, refusing an unknown field or a value of the wrong type; dynamic execute refuses unknown and missing params before that, and meta holds `params` to the action's schema only in the `full` and `compact` modes of `GITLAB_MCP_META_PARAM_SCHEMA`, not in the default `opaque` one. Arguments nested past `toolutil.DefaultMaxArgumentDepth` (64) are refused before the schema decode, and in HTTP mode a request body nested past `maxInboundJSONDepth` (100, `cmd/server/main.go`) before anything reads it | `TestUnmarshalParams_RejectsUnknownField`, `TestUnmarshalParams_InvalidType`, `TestMetaTool_InlineModes_HoldParamsToTheActionsDefinition` and `TestAttachArgumentLimits_RefusesOverNestedArguments` (`internal/toolutil`); `TestExecute_ReportsUnknownAndMissingParamsBeforeDispatch` (`internal/tools/dynamic`); `TestServeHTTP_RefusesABodyNestedPastTheInboundDepth` and `TestSecurityHeaders_RefusesDeeplyNestedBody` (`cmd/server`) | Holds for the decoding and the depth limits. Not traced: the SDK's validation on the individual surface, which no test here holds, and the statements "Integer IDs are validated to prevent injection" and "String parameters are sanitized where applicable" | | Error handling | The four wrappers of `internal/toolutil/errors.go`, `NotFoundResult` and the panic backstop above | `TestWrapErr_AddsContextAndClassification` and the `TestClassifyError_*` cases (`internal/toolutil/errors_test.go`), among them `TestClassifyError_CredentialRejected_NamesTheTokenAlone`; `TestRecoverPanics_KeepsTheProcessAndHidesTheStack` (`cmd/server`) | Holds for the wrappers and the backstop. Not traced: that a wrapped GitLab error carries no request header | -| Dependencies | govulncheck and `make check-binary-vulns` on every pull request and push to `main`, `check-binary-vulns` again on the tagged tree before a release publishes anything; Dependabot | The `govulncheck` job of CI, which runs govulncheck and `make check-binary-vulns`, and the same `make check-binary-vulns` step in `release.yml`, each of which fails on a finding | The 3.3.0 artifacts carry advisories fixed since, see [Findings](#findings) | +| Dependencies | govulncheck and `make check-binary-vulns` on every pull request and push to `main`, `check-binary-vulns` again on the tagged tree before a release publishes anything; Dependabot | The `govulncheck` job of CI, which runs govulncheck and `make check-binary-vulns`, and the same `make check-binary-vulns` step in `release.yml`, each of which fails on a finding | The 3.3.0 artifacts carry advisories fixed since; 3.4.0 ships those fixes, see findings 4 and 5 in [Findings](#findings) | | Release integrity | GoReleaser builds from the tag, cosign signs `checksums.txt` with the workflow's keyless identity, immutable releases are on and every release from 3.0.0 is immutable, and every tag from v2.7.0 on is signed | The `verify-published` job of `release.yml`, which verifies the published `checksums.txt` against its signature, and the registry packages against those checksums, before Homebrew, the MCP Registry and winget are pointed at the release; rule 5 of `make check-supply-chain`, which holds both installers to verifying that signature; `make check-server-json-packages`, which warns when the latest release is mutable | Holds for the checksums, their signature and immutability. Not traced: the tag signatures, which were read for this record and which no check holds | | The HTTP listener | `--http-addr` serves plain HTTP unless `--tls-cert` and `--tls-key` are given, and the image serves it on every interface by default; startup warns about such a listener off loopback (`cmd/server.checkPlaintextListener`) unless `--allow-plaintext-http` declares a proxied hop, and `--allow-plaintext-http=false` is the hardened mode, which refuses to start it (`ADM-015`) | `TestCheckPlaintextListener_SaysWhatTheListenerExposes` (`cmd/server`), and `TestListener_PlaintextOffLoopback_WarnsAtStartup`, `TestListener_PlaintextOffLoopback_TheOptInIsReadFromTheFlagAndTheVariable`, `TestListener_PlaintextOffLoopback_TheHardenedModeRefusesToStart` and `TestListener_ListenersThatCrossNoNetworkInTheClear_AreNotAnnounced` (`test/e2e/http`). By default nothing refuses the listener: that is finding 6 | Not as the OpenSSF criterion `crypto_used_network` asks, see [Findings](#findings) | @@ -185,16 +194,16 @@ says what the rule sets and why. ## Findings -| # | Finding | Severity | Status | -| --- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| 1 | [GHSA-fcj2-hj27-hj26](https://github.com/jmrplens/gitlab-mcp-server/security/advisories/GHSA-fcj2-hj27-hj26), reported on 12 June 2026: legacy HTTP mode sent requests, with whatever token the caller supplied, to any host the `GITLAB-URL` header named (CWE-918), from 1.1.0 through 2.7.5 | High | Fixed in 3.0.0 (10 September 2026) by #479 and #645, the advisory published on 14 September; boundary 1 above holds the fix. The advisory is not in the GitHub Advisory Database or OSV yet, so scanners do not report it, and the CVE requested on 9 September is not assigned; both are followed in the issue *GHSA-fcj2-hj27-hj26 is missing from the GitHub Advisory Database and OSV, and its CVE is unassigned* | -| 2 | Code scanning alerts [9](https://github.com/jmrplens/gitlab-mcp-server/security/code-scanning/9) and [12](https://github.com/jmrplens/gitlab-mcp-server/security/code-scanning/12) (`go/request-forgery`, `internal/gitlab/client.go`, raised in April and August 2026) are dismissed as false positives, and they were the flow of finding 1 before 3.0.0 closed it; alerts [3](https://github.com/jmrplens/gitlab-mcp-server/security/code-scanning/3), [6](https://github.com/jmrplens/gitlab-mcp-server/security/code-scanning/6) and [7](https://github.com/jmrplens/gitlab-mcp-server/security/code-scanning/7) (`go/path-injection`, `internal/wizard/install.go`) are dismissed as false positives, where the flow of alert 3 went with pull request #213 on 6 July 2026 and the code of alerts 6 and 7 was removed in pull request #336 | Low: the record is wrong, the code is fixed | Open: GitHub already records each of the five as no longer found, and reopening them lets the next analysis close them as fixed; any it still finds is dismissed again with its true reason. That is a repository action, the maintainer's | -| 3 | SECURITY.md introduces its trust boundaries with "Three of them matter" and lists four; it offers a "direct message on GitHub" as the private channel beside security advisories, which GitHub does not have; its coordinated disclosure says a fix is developed in a private fork, where the fix of finding 1 was developed in public pull requests; and finding 1 was fixed 90 days after it was reported, where the policy's target for a High is 30 days from confirmation | Low: the policy misstates what happens | In the issue *Make the security policy describe what happens, and publish an assurance case*, whose pull request corrects the three statements, replaces the seven-day embargo with what happened, and states finding 1's 90 days beside the targets; whether the targets stay as they are is the maintainer's | -| 4 | The 3.3.0 image is built on `alpine:3.24.2`, whose zlib carries CVE-2026-85091 and whose musl carries CVE-2026-107891, both fixed in Alpine 3.24.3 | Medium for zlib; musl's is not rated yet | In the issue *Take Alpine 3.24.3 in the image: zlib CVE-2026-85091 and musl CVE-2026-107891*, whose `Dockerfile` change is its own pull request; it reaches users with a release, which has to come before 2 November 2026, sixty days after CVE-2026-85091 was published | -| 5 | The 3.3.0 binaries are built with go1.27.2, whose standard library has advisories fixed in go1.27.3 (GO-2026-6809 among them) | As the advisories rate them | Fixed on `main`, which builds with go1.27.3; it reaches users with the next release | -| 6 | A TCP listener off loopback serves plain HTTP unless TLS is configured, and the image serves it on every interface by default; the OpenSSF gold criterion `crypto_used_network` asks that such protocols are off by default | Design: most deployments terminate TLS in a proxy | Accepted as a design choice on 27 October 2026, after [Refusing Plain HTTP off Loopback](plaintext-http-refusal-analysis.md): plain HTTP stays served by default, startup warns about such a listener and names the hardened mode, `--allow-plaintext-http` declares a proxied hop, and `--allow-plaintext-http=false` or `GITLAB_MCP_ALLOW_PLAINTEXT_HTTP=false` refuses to start it. The gold criterion stays unmet by that choice, which the issue *Refuse a plaintext HTTP listener off loopback unless TLS or an explicit opt-in is given* records | -| 7 | The documentation site sends no `Content-Security-Policy` and no `X-Frame-Options` | Low: a static site with no sign-in | The check is in place (above); the headers wait for the Cloudflare rule, in the issue *Record a security review, and send CSP and X-Frame-Options from the documentation site* | -| 8 | [`GHSA-rxjw-584q-hxjr`](https://github.com/jmrplens/gitlab-mcp-server/security/advisories/GHSA-rxjw-584q-hxjr): a tool argument placed into the path of a request to GitLab was not checked for path separators or dot segments, so a crafted value could make the server build a request path that, once a front layer or GitLab resolves the dot segments, names a different GitLab API endpoint than the action intends, under the caller's token (CWE-23, CWE-20). Affects the job artifact download, model registry download, pipeline schedule variable and template actions from 1.0.0, the system hook URL-variable actions from 2.0.2, the group hook URL-variable, custom-header and test actions from 2.3.0, and from 1.0.0 any action whose escaped path argument is `.` or `..` as a whole, all through 3.3.0. A standard GitLab install does not serve it (`Gitlab::Middleware::PathTraversalCheck` answers 400 by default from GitLab 18.2; before it the check is off by default, and the request is answered 404 unless its feature flags are switched on); a front layer that normalizes the path before forwarding lets it through. The root cause is client-go's `NoEscape` on these path variables, which was reported to GitLab privately, and, for a whole `.` or `..`, this server's own rewrite of `%2E` to `.` and the model registry route's `url.PathEscape`, which both leave it a dot segment. The model registry `path` stays affected on 3.3.0, whose `path.Clean` normalization resolves a `.` or `..` between slashes but keeps one marked off by a backslash as a name | Medium | Fixed in 3.4.0 by [`39b7093af`](https://github.com/jmrplens/gitlab-mcp-server/commit/39b7093afffede232184c1e385b1ef4a215b0258); advisory published on 31 October 2026. The boundary 1 path row above holds the fix | +| # | Finding | Severity | Status | +| --- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | [GHSA-fcj2-hj27-hj26](https://github.com/jmrplens/gitlab-mcp-server/security/advisories/GHSA-fcj2-hj27-hj26), reported on 12 June 2026: legacy HTTP mode sent requests, with whatever token the caller supplied, to any host the `GITLAB-URL` header named (CWE-918), from 1.1.0 through 2.7.5 | High | Fixed in 3.0.0 (10 September 2026) by #479 and #645, the advisory published on 14 September; boundary 1 above holds the fix. The advisory is not in the GitHub Advisory Database or OSV yet, so scanners do not report it, and the CVE requested on 9 September is not assigned; both are followed in the issue *GHSA-fcj2-hj27-hj26 is missing from the GitHub Advisory Database and OSV, and its CVE is unassigned* | +| 2 | Code scanning alerts [9](https://github.com/jmrplens/gitlab-mcp-server/security/code-scanning/9) and [12](https://github.com/jmrplens/gitlab-mcp-server/security/code-scanning/12) (`go/request-forgery`, `internal/gitlab/client.go`, raised in April and August 2026) are dismissed as false positives, and they were the flow of finding 1 before 3.0.0 closed it; alerts [3](https://github.com/jmrplens/gitlab-mcp-server/security/code-scanning/3), [6](https://github.com/jmrplens/gitlab-mcp-server/security/code-scanning/6) and [7](https://github.com/jmrplens/gitlab-mcp-server/security/code-scanning/7) (`go/path-injection`, `internal/wizard/install.go`) are dismissed as false positives, where the flow of alert 3 went with pull request #213 on 6 July 2026 and the code of alerts 6 and 7 was removed in pull request #336 | Low: the record is wrong, the code is fixed | Open: GitHub already records each of the five as no longer found, and reopening them lets the next analysis close them as fixed; any it still finds is dismissed again with its true reason. That is a repository action, the maintainer's | +| 3 | SECURITY.md introduces its trust boundaries with "Three of them matter" and lists four; it offers a "direct message on GitHub" as the private channel beside security advisories, which GitHub does not have; its coordinated disclosure says a fix is developed in a private fork, where the fix of finding 1 was developed in public pull requests; and finding 1 was fixed 90 days after it was reported, where the policy's target for a High is 30 days from confirmation | Low: the policy misstates what happens | In the issue *Make the security policy describe what happens, and publish an assurance case*, whose pull request corrects the three statements, replaces the seven-day embargo with what happened, and states finding 1's 90 days beside the targets; whether the targets stay as they are is the maintainer's. Update, 31 October 2026: corrected on `main` by #1560, merged on 28 October 2026, which closed the issue, with the targets left as they were; its account of how a fix is developed, in pull requests of this repository, and of when the advisory follows the release, a few days after it, did not yet describe finding 8, whose fix was merged from the advisory's private fork and whose advisory was published the day 3.4.0 was released | +| 4 | The 3.3.0 image is built on `alpine:3.24.2`, whose zlib carries CVE-2026-85091 and whose musl carries CVE-2026-107891, both fixed in Alpine 3.24.3 | Medium for zlib; musl's is not rated yet | In the issue *Take Alpine 3.24.3 in the image: zlib CVE-2026-85091 and musl CVE-2026-107891*, whose `Dockerfile` change is its own pull request; it reaches users with a release, which has to come before 2 November 2026, sixty days after CVE-2026-85091 was published. Update, 31 October 2026: fixed by #1554, merged on 28 October 2026, which closed the issue, and released in 3.4.0 on 31 October 2026, before that deadline | +| 5 | The 3.3.0 binaries are built with go1.27.2, whose standard library has advisories fixed in go1.27.3 (GO-2026-6809 among them) | As the advisories rate them | Fixed on `main`, which builds with go1.27.3; it reaches users with the next release. Update, 31 October 2026: released in 3.4.0 on 31 October 2026, whose binaries are built with go1.27.3 | +| 6 | A TCP listener off loopback serves plain HTTP unless TLS is configured, and the image serves it on every interface by default; the OpenSSF gold criterion `crypto_used_network` asks that such protocols are off by default | Design: most deployments terminate TLS in a proxy | Accepted as a design choice on 27 October 2026, after [Refusing Plain HTTP off Loopback](plaintext-http-refusal-analysis.md): plain HTTP stays served by default, startup warns about such a listener and names the hardened mode, `--allow-plaintext-http` declares a proxied hop, and `--allow-plaintext-http=false` or `GITLAB_MCP_ALLOW_PLAINTEXT_HTTP=false` refuses to start it. The gold criterion stays unmet by that choice, which the issue *Refuse a plaintext HTTP listener off loopback unless TLS or an explicit opt-in is given* records | +| 7 | The documentation site sends no `Content-Security-Policy` and no `X-Frame-Options` | Low: a static site with no sign-in | The check is in place (above); the headers wait for the Cloudflare rule, in the issue *Record a security review, and send CSP and X-Frame-Options from the documentation site* | +| 8 | [`GHSA-rxjw-584q-hxjr`](https://github.com/jmrplens/gitlab-mcp-server/security/advisories/GHSA-rxjw-584q-hxjr): a tool argument placed into the path of a request to GitLab was not checked for path separators or dot segments, so a crafted value could make the server build a request path that, once a front layer or GitLab resolves the dot segments, names a different GitLab API endpoint than the action intends, under the operator's token, or in HTTP mode the caller's, never beyond what that token may do but past the server's per-action controls: the tool exclusion list, the tier filter, the chosen tool surface and the destructive-action confirmation (CWE-23, CWE-20). Affects the job artifact download, model registry download, pipeline schedule variable and template actions from 1.0.0, the system hook URL-variable actions from 2.0.2, the group hook URL-variable, custom-header and test actions from 2.3.0, and from 1.0.0 any action whose escaped path argument is `.` or `..` as a whole, all through 3.3.0. A standard GitLab install does not serve it (`Gitlab::Middleware::PathTraversalCheck` answers 400 by default from GitLab 18.2; before it its `check_path_traversal_middleware` flag is off by default, and before 16.5 there is no check; switched on, it refuses on 18.0 and 18.1, on 17.4 to 17.11 only with `check_path_traversal_middleware_reject_requests` on as well, and from 16.5 to 17.3 it can only log; wherever it does not refuse, the request is answered 404); a front layer that normalizes the path before forwarding lets it through. The root cause is client-go's `NoEscape` on these path variables, which was reported to GitLab privately, and, for a whole `.` or `..`, this server's own rewrite of `%2E` to `.` and the model registry route's `url.PathEscape`, which both leave it a dot segment. The model registry `path` stays affected on 3.3.0, whose `path.Clean` normalization resolves a `.` or `..` between slashes but keeps one marked off by a backslash as a name | Medium | Fixed in [3.4.0](https://github.com/jmrplens/gitlab-mcp-server/releases/tag/v3.4.0) (31 October 2026) by [`39b7093af`](https://github.com/jmrplens/gitlab-mcp-server/commit/39b7093afffede232184c1e385b1ef4a215b0258), merged from the advisory's temporary private fork, so no pull request of this repository carries it; the advisory was published the same day, once the release was out. The boundary 1 path row above holds the fix. The advisory is not in the GitHub Advisory Database or OSV yet, so scanners do not report it, and the CVE requested before it was published is not assigned | ## What the maintainer's review covers From 73ade1dc18baa3f8b1a83fce8b00f209d8e3b3df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Sat, 31 Oct 2026 08:44:13 +0100 Subject: [PATCH 2/2] docs(security): sign off the October review, finding 8 included MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The record still read as if finding 8 were held back until its advisory is published, and asked whether it changes the result of any row without answering. GHSA-rxjw-584q-hxjr was published on 31 October 2026 with 3.4.0, and I read the boundary 1 path row and finding 8 that day. The status line, the introduction and the Sign-off table now say the review is signed off as of 31 October 2026, every finding included, with finding 8 held back from the sign-off of 27 October until its advisory was published, and the path row read at 39b7093af, the commit 3.4.0 was released from. What the maintainer's review covers now answers the question 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 that row now says so. The others keep their results, because 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. Signed-off-by: José M. Requena Plens --- docs/development/security-review-2026-10.md | 81 +++++++++++++-------- 1 file changed, 51 insertions(+), 30 deletions(-) diff --git a/docs/development/security-review-2026-10.md b/docs/development/security-review-2026-10.md index 4b1d7e7eb..d0dfc9e3b 100644 --- a/docs/development/security-review-2026-10.md +++ b/docs/development/security-review-2026-10.md @@ -3,17 +3,19 @@ > **Diátaxis type**: Explanation > **Audience**: 🛠️ Maintainers, and anyone assessing how the project's security is reviewed -**Status: reviewed and signed off by the maintainer in October 2026.** +**Status: reviewed and signed off by the maintainer, as of 31 October 2026 with finding 8 included.** Prepared on 26 October 2026 from `main` at [`8e959c3d8`](https://github.com/jmrplens/gitlab-mcp-server/commit/8e959c3d8e70dc384931b49a66bce00c5f2ec8fa) and from the repository's security state on GitHub that day. The criterion this record answers requires a review by a person as well as by tools: the maintainer read the record against the code at that commit and signed it off -on 27 October 2026, as [Sign-off](#sign-off) records, with finding 8 held back -under coordinated disclosure until its advisory is published. The advisory, -GHSA-rxjw-584q-hxjr, was published on 31 October 2026 with 3.4.0, and the two -rows it held back, the boundary 1 path row and finding 8, were added then. The -path row's Enforced by and Held by columns describe the code at +on 27 October 2026, with finding 8 held back under coordinated disclosure until +its advisory was published. The advisory, GHSA-rxjw-584q-hxjr, was published on +31 October 2026 with 3.4.0, and the two rows it held back, the boundary 1 path +row and finding 8, were added then; the maintainer read them that day and +signed off the review as of 31 October 2026, every finding included, as +[Sign-off](#sign-off) records. The path row's Enforced by and Held by columns +describe the code at [`39b7093af`](https://github.com/jmrplens/gitlab-mcp-server/commit/39b7093afffede232184c1e385b1ef4a215b0258), the commit 3.4.0 was released from, since the requirement did not hold at the commit reviewed, and finding 8 was checked against the releases its advisory @@ -40,9 +42,9 @@ finding is tracked. ## Scope - **Reviewed:** the server, `cmd/server` and `internal/`, at the commit above, - and for the boundary 1 path row at `39b7093af`; - its release configuration (`.goreleaser.yml`, `.github/workflows/release.yml` - and the `Dockerfile`) and what the 3.3.0 release published from it; the + and for the boundary 1 path row at `39b7093af`; its release configuration + (`.goreleaser.yml`, `.github/workflows/release.yml` and the `Dockerfile`) + and what the 3.3.0 release published from it; the repository's code scanning, Dependabot and secret scanning alerts; and the response headers of the documentation site. - **Against:** the statements of SECURITY.md's @@ -65,9 +67,9 @@ finding is tracked. **Tracing.** Each requirement was traced to the code that enforces it, read at the commit (the boundary 1 path row excepted, which was traced at `39b7093af`, as the introduction says), and to the tests or gates that would fail if it -stopped holding. A -requirement is recorded as holding only when both exist, and a statement this -preparation found no code or no test for is recorded as not traced. +stopped holding. A requirement is recorded as holding only when both exist, +and a statement this preparation found no code or no test for is recorded as +not traced. **Automated analysis.** What GitHub ran on the commit: @@ -154,15 +156,15 @@ that no row rests on more is the maintainer's. ## Security considerations -| Statement of SECURITY.md | Enforced by | Held by | Result | -| -------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Token handling | Boundaries 1 and 3, and credential disclosure above | The rows it names | Holds | -| TLS: verification on by default, `GITLAB_MCP_SKIP_TLS_VERIFY` only where trusted | The GitLab client verifies certificates unless the skip is set, and it, the HTTPS listener and the health probe set TLS 1.2 as their minimum (`internal/gitlab/client.go`, `cmd/server/tls_reload.go`, `cmd/server/probe.go`); OAuth mode refuses the skip for an instance off loopback | `TestBuildDestinationPools_Verified_OnePairSharedAndLent` and `TestBuildDestinationPools_SkipTLSVerify_APairOfTheClientsOwn` (`internal/gitlab`); `TestTLSConfigFor_StatesTheFloorAndReloads`, `TestProbeTLSConfig_HoldsTheServersFloor` and `TestValidateHTTPAuthConfig_OAuthRefusesSkipTLSVerify` (`cmd/server`); `TestListener_TLS_RefusesObsoleteVersions` and `TestOAuth_SkipTLSVerifyScope` (`test/e2e/http`) | Holds | -| Input validation | On the individual surface the SDK validates each call against the action's input schema before a handler runs. On the dynamic and meta surfaces it validates the envelope, and `toolutil.UnmarshalParams` decodes `params` into the action's typed input, refusing an unknown field or a value of the wrong type; dynamic execute refuses unknown and missing params before that, and meta holds `params` to the action's schema only in the `full` and `compact` modes of `GITLAB_MCP_META_PARAM_SCHEMA`, not in the default `opaque` one. Arguments nested past `toolutil.DefaultMaxArgumentDepth` (64) are refused before the schema decode, and in HTTP mode a request body nested past `maxInboundJSONDepth` (100, `cmd/server/main.go`) before anything reads it | `TestUnmarshalParams_RejectsUnknownField`, `TestUnmarshalParams_InvalidType`, `TestMetaTool_InlineModes_HoldParamsToTheActionsDefinition` and `TestAttachArgumentLimits_RefusesOverNestedArguments` (`internal/toolutil`); `TestExecute_ReportsUnknownAndMissingParamsBeforeDispatch` (`internal/tools/dynamic`); `TestServeHTTP_RefusesABodyNestedPastTheInboundDepth` and `TestSecurityHeaders_RefusesDeeplyNestedBody` (`cmd/server`) | Holds for the decoding and the depth limits. Not traced: the SDK's validation on the individual surface, which no test here holds, and the statements "Integer IDs are validated to prevent injection" and "String parameters are sanitized where applicable" | -| Error handling | The four wrappers of `internal/toolutil/errors.go`, `NotFoundResult` and the panic backstop above | `TestWrapErr_AddsContextAndClassification` and the `TestClassifyError_*` cases (`internal/toolutil/errors_test.go`), among them `TestClassifyError_CredentialRejected_NamesTheTokenAlone`; `TestRecoverPanics_KeepsTheProcessAndHidesTheStack` (`cmd/server`) | Holds for the wrappers and the backstop. Not traced: that a wrapped GitLab error carries no request header | -| Dependencies | govulncheck and `make check-binary-vulns` on every pull request and push to `main`, `check-binary-vulns` again on the tagged tree before a release publishes anything; Dependabot | The `govulncheck` job of CI, which runs govulncheck and `make check-binary-vulns`, and the same `make check-binary-vulns` step in `release.yml`, each of which fails on a finding | The 3.3.0 artifacts carry advisories fixed since; 3.4.0 ships those fixes, see findings 4 and 5 in [Findings](#findings) | -| Release integrity | GoReleaser builds from the tag, cosign signs `checksums.txt` with the workflow's keyless identity, immutable releases are on and every release from 3.0.0 is immutable, and every tag from v2.7.0 on is signed | The `verify-published` job of `release.yml`, which verifies the published `checksums.txt` against its signature, and the registry packages against those checksums, before Homebrew, the MCP Registry and winget are pointed at the release; rule 5 of `make check-supply-chain`, which holds both installers to verifying that signature; `make check-server-json-packages`, which warns when the latest release is mutable | Holds for the checksums, their signature and immutability. Not traced: the tag signatures, which were read for this record and which no check holds | -| The HTTP listener | `--http-addr` serves plain HTTP unless `--tls-cert` and `--tls-key` are given, and the image serves it on every interface by default; startup warns about such a listener off loopback (`cmd/server.checkPlaintextListener`) unless `--allow-plaintext-http` declares a proxied hop, and `--allow-plaintext-http=false` is the hardened mode, which refuses to start it (`ADM-015`) | `TestCheckPlaintextListener_SaysWhatTheListenerExposes` (`cmd/server`), and `TestListener_PlaintextOffLoopback_WarnsAtStartup`, `TestListener_PlaintextOffLoopback_TheOptInIsReadFromTheFlagAndTheVariable`, `TestListener_PlaintextOffLoopback_TheHardenedModeRefusesToStart` and `TestListener_ListenersThatCrossNoNetworkInTheClear_AreNotAnnounced` (`test/e2e/http`). By default nothing refuses the listener: that is finding 6 | Not as the OpenSSF criterion `crypto_used_network` asks, see [Findings](#findings) | +| Statement of SECURITY.md | Enforced by | Held by | Result | +| -------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Token handling | Boundaries 1 and 3, and credential disclosure above | The rows it names | Holds | +| TLS: verification on by default, `GITLAB_MCP_SKIP_TLS_VERIFY` only where trusted | The GitLab client verifies certificates unless the skip is set, and it, the HTTPS listener and the health probe set TLS 1.2 as their minimum (`internal/gitlab/client.go`, `cmd/server/tls_reload.go`, `cmd/server/probe.go`); OAuth mode refuses the skip for an instance off loopback | `TestBuildDestinationPools_Verified_OnePairSharedAndLent` and `TestBuildDestinationPools_SkipTLSVerify_APairOfTheClientsOwn` (`internal/gitlab`); `TestTLSConfigFor_StatesTheFloorAndReloads`, `TestProbeTLSConfig_HoldsTheServersFloor` and `TestValidateHTTPAuthConfig_OAuthRefusesSkipTLSVerify` (`cmd/server`); `TestListener_TLS_RefusesObsoleteVersions` and `TestOAuth_SkipTLSVerifyScope` (`test/e2e/http`) | Holds | +| Input validation | On the individual surface the SDK validates each call against the action's input schema before a handler runs. On the dynamic and meta surfaces it validates the envelope, and `toolutil.UnmarshalParams` decodes `params` into the action's typed input, refusing an unknown field or a value of the wrong type; dynamic execute refuses unknown and missing params before that, and meta holds `params` to the action's schema only in the `full` and `compact` modes of `GITLAB_MCP_META_PARAM_SCHEMA`, not in the default `opaque` one. Arguments nested past `toolutil.DefaultMaxArgumentDepth` (64) are refused before the schema decode, and in HTTP mode a request body nested past `maxInboundJSONDepth` (100, `cmd/server/main.go`) before anything reads it | `TestUnmarshalParams_RejectsUnknownField`, `TestUnmarshalParams_InvalidType`, `TestMetaTool_InlineModes_HoldParamsToTheActionsDefinition` and `TestAttachArgumentLimits_RefusesOverNestedArguments` (`internal/toolutil`); `TestExecute_ReportsUnknownAndMissingParamsBeforeDispatch` (`internal/tools/dynamic`); `TestServeHTTP_RefusesABodyNestedPastTheInboundDepth` and `TestSecurityHeaders_RefusesDeeplyNestedBody` (`cmd/server`) | Holds for the decoding and the depth limits. Not traced: the SDK's validation on the individual surface, which no test here holds, and the statements "Integer IDs are validated to prevent injection" and "String parameters are sanitized where applicable". The second of those did not hold for a path argument until 3.4.0, finding 8. Update, 31 October 2026: the SDK's validation on the individual surface is held since then by `TestIndividualSurface_ArgumentsTheSchemaRefuses_NeverReachGitLab` (`test/e2e/stdio`), which drives the real binary | +| Error handling | The four wrappers of `internal/toolutil/errors.go`, `NotFoundResult` and the panic backstop above | `TestWrapErr_AddsContextAndClassification` and the `TestClassifyError_*` cases (`internal/toolutil/errors_test.go`), among them `TestClassifyError_CredentialRejected_NamesTheTokenAlone`; `TestRecoverPanics_KeepsTheProcessAndHidesTheStack` (`cmd/server`) | Holds for the wrappers and the backstop. Not traced: that a wrapped GitLab error carries no request header | +| Dependencies | govulncheck and `make check-binary-vulns` on every pull request and push to `main`, `check-binary-vulns` again on the tagged tree before a release publishes anything; Dependabot | The `govulncheck` job of CI, which runs govulncheck and `make check-binary-vulns`, and the same `make check-binary-vulns` step in `release.yml`, each of which fails on a finding | The 3.3.0 artifacts carry advisories fixed since; 3.4.0 ships those fixes, see findings 4 and 5 in [Findings](#findings) | +| Release integrity | GoReleaser builds from the tag, cosign signs `checksums.txt` with the workflow's keyless identity, immutable releases are on and every release from 3.0.0 is immutable, and every tag from v2.7.0 on is signed | The `verify-published` job of `release.yml`, which verifies the published `checksums.txt` against its signature, and the registry packages against those checksums, before Homebrew, the MCP Registry and winget are pointed at the release; rule 5 of `make check-supply-chain`, which holds both installers to verifying that signature; `make check-server-json-packages`, which warns when the latest release is mutable | Holds for the checksums, their signature and immutability. Not traced: the tag signatures, which were read for this record and which no check holds | +| The HTTP listener | `--http-addr` serves plain HTTP unless `--tls-cert` and `--tls-key` are given, and the image serves it on every interface by default; startup warns about such a listener off loopback (`cmd/server.checkPlaintextListener`) unless `--allow-plaintext-http` declares a proxied hop, and `--allow-plaintext-http=false` is the hardened mode, which refuses to start it (`ADM-015`) | `TestCheckPlaintextListener_SaysWhatTheListenerExposes` (`cmd/server`), and `TestListener_PlaintextOffLoopback_WarnsAtStartup`, `TestListener_PlaintextOffLoopback_TheOptInIsReadFromTheFlagAndTheVariable`, `TestListener_PlaintextOffLoopback_TheHardenedModeRefusesToStart` and `TestListener_ListenersThatCrossNoNetworkInTheClear_AreNotAnnounced` (`test/e2e/http`). By default nothing refuses the listener: that is finding 6 | Not as the OpenSSF criterion `crypto_used_network` asks, see [Findings](#findings) | ## The project's websites @@ -209,7 +211,8 @@ says what the rule sets and why. What tools cannot find is the part of the review that rests on a person, the design above all. The maintainer's reading takes each row above against the -code at the commit, and in particular: +code at the commit, the boundary 1 path row against the code at `39b7093af`, +and in particular: - whether the four boundaries are still the ones that matter, and whether anything has crossed one of them since the September audit without a test @@ -220,15 +223,33 @@ code at the commit, and in particular: - the statements recorded as not traced, and whether each is enforced somewhere this preparation did not find, should be, or should leave SECURITY.md; -- finding 8, once its advisory is published, and whether it changes the - result of any row above; +- finding 8, held back until GHSA-rxjw-584q-hxjr was published on 31 October + 2026, and whether it changes the result of any row above. It adds the + boundary 1 path row it held back, and of the rows recorded on 27 October it + moves only the input validation row, whose untraced statement "String + parameters are sanitized where applicable" did not hold for a path argument + until 3.4.0. The others keep their results: every request still ran under + the token's own authority and never beyond its scope and grants; read-only + mode removed every write action finding 8 reaches and safe mode turned each + into a preview before any request was built, while a read action's + redirected request stayed a GET; and the request stayed on the instance. So + boundary 1's first three rows hold, and with them the rows that rest on + boundary 1: credential disclosure, privilege escalation past the mode, + server-side request forgery and token handling. Path traversal on file + operations is about local paths. Finding 8 falls under boundary 1's path + clause and under none of the six kinds SECURITY.md named on 27 October, + and the per-action controls its advisory says it bypassed were not a + requirement SECURITY.md stated; the maintainer decided on 31 October 2026 + that SECURITY.md names both, the request-path traversal as a kind of + vulnerability and the per-action controls as a requirement, and it does + from then on; - whether each severity in [Findings](#findings) is the right one. ## Sign-off -| Field | Value | -| ------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------- | -| Reviewed by | The maintainer, José M. Requena Plens ([@jmrplens](https://github.com/jmrplens)) | -| Date of the review | 27 October 2026 | -| Commit reviewed | [`8e959c3d8`](https://github.com/jmrplens/gitlab-mcp-server/commit/8e959c3d8e70dc384931b49a66bce00c5f2ec8fa), `main` on 26 October 2026 | -| Result | As recorded, with finding 8 held back under [coordinated disclosure](../../SECURITY.md#coordinated-disclosure) until its advisory is published | +| Field | Value | +| ------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Reviewed by | The maintainer, José M. Requena Plens ([@jmrplens](https://github.com/jmrplens)) | +| Date of the review | 27 October 2026, and 31 October 2026 for the boundary 1 path row and finding 8 | +| Commit reviewed | [`8e959c3d8`](https://github.com/jmrplens/gitlab-mcp-server/commit/8e959c3d8e70dc384931b49a66bce00c5f2ec8fa), `main` on 26 October 2026; for the boundary 1 path row, [`39b7093af`](https://github.com/jmrplens/gitlab-mcp-server/commit/39b7093afffede232184c1e385b1ef4a215b0258), the commit 3.4.0 was released from | +| Result | As recorded, signed off on 31 October 2026 with every finding, finding 8 included. It was held back under [coordinated disclosure](../../SECURITY.md#coordinated-disclosure) from the sign-off of 27 October 2026 until its advisory, GHSA-rxjw-584q-hxjr, was published on 31 October 2026, the day 3.4.0 released its fix |