fix(proxy): a *.railway.internal upstream is bearer-safe over HTTP - #127
Closed
jbdevprimary wants to merge 13 commits into
Closed
jbdevprimary wants to merge 13 commits into
jbdevprimary wants to merge 13 commits into
Conversation
…omputed hash GitHub sends X-Hub-Signature-256 on every delivery and its documentation is explicit that receivers should use it; this proxy validated only the SHA-1 header. On the one process a webhook proxy deliberately exposes to the internet, that is weaker checking than the payload already permits. SHA-256 is now preferred with SHA-1 as the fallback. The fallback is deliberate rather than reluctant: GitLab and older GitHub Enterprise installations send SHA-1 only, and dropping it would break working deployments for no security gain when SHA-256 is absent. WHEN BOTH HEADERS ARE PRESENT THE SHA-256 ONE DECIDES, and that is the part worth stating. A receiver that fell back to SHA-1 on a BAD SHA-256 would let an attacker choose the weaker algorithm by pairing a valid SHA-1 with a forged SHA-256. There is a test for exactly that case. AND THE COMPUTED HASH IS NO LONGER LOGGED. `IsValidPayload` printed it on every delivery. The digest is derived from the shared secret over a body an attacker can choose, so publishing it to logs hands out an oracle for free — and it debugs nothing the rejection itself does not already show. TestGithubProvider_Validate was entirely commented out, which is why the implementation carried a `// TODO: Update implementation and tests`. It is now eleven real cases covering both algorithms, the downgrade attempt, a tampered payload, a wrong secret, malformed prefixes and lengths, nil headers, and an empty SHA-256 header falling through to SHA-1. Signatures are COMPUTED from the helpers rather than pasted, so a change to either helper fails the tests instead of quietly agreeing with itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…se to Go 1.25 TWO CHANGES, ONE OF THEM A SECURITY FIX. CONSTANT-TIME TOKEN COMPARISON. `GitlabProvider.Validate` compared `X-Gitlab-Token` against the configured secret with `==`. Go's string comparison returns as soon as two bytes differ, so how long a rejection takes is a function of how many leading characters the caller guessed correctly. A webhook endpoint is public and accepts as many requests as you send it, which is exactly the condition a timing attack needs to recover a token one byte at a time. Unlike GitHub, GitLab sends the token VERBATIM rather than a signature — there is no digest to compare, so this string IS the credential and the comparison is the whole of the check. `subtle.ConstantTimeCompare` also returns 0 for differing lengths without examining contents, which is correct here. Three tests were added for the shapes an attack actually uses: a prefix of the secret, a length-equal near-miss with the last byte changed, and the surrounding-whitespace case, which is trimmed deliberately — some proxies pad the header and rejecting those would break working deployments. GO 1.25, UP FROM 1.13. The upstream project's last release was November 2020 and its last commit March 2023, so it was pinned to a toolchain that had already left support. Thirteen releases of security fixes sat on the other side of a one-line bump. `httpmock` moves 1.0.4 -> 1.4.1 with it. MODULE PATH IS NOW `github.com/jbcom/GitWebhookProxy`. Renaming without rewriting the internal imports made `go mod tidy` pull the UPSTREAM module in as a dependency of the fork — a fork that silently compiles half of the code it replaced. All five importing files were rewritten; upstream is no longer in `go.mod` at all. go build, go vet and the full test suite pass on Go 1.26. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fork's first substantive change, and the reason it exists. Upstream's last
release was November 2020 and its last commit March 2023; two of the three
fixes here are security fixes on the one process this project deliberately
exposes to the internet.
* GitHub deliveries are validated against X-Hub-Signature-256, with SHA-1
kept as a fallback for GitLab and older GitHub Enterprise. When both
headers are present the SHA-256 one decides, so an attacker cannot
downgrade by pairing a valid SHA-1 with a forged SHA-256.
* The computed HMAC is no longer logged on every delivery.
* GitLab's token comparison is constant-time. It was `==`, on a credential
sent verbatim to a public endpoint that accepts unlimited attempts.
* Go 1.13 -> 1.25, and the module path is the fork's own.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…and build from source
THE HOLE. `proxyRequest` ran the ignored/allowed-user filter first and answered
200 to anything it decided to ignore — before the signature was ever checked.
`isIgnoredUser` treats an EMPTY committer on the github provider as ignored, so
an UNSIGNED request reaching that branch got a 200 from a proxy that had never
authenticated it.
Measured against the built image, `GWP_SECRET` set, no signature header at all:
Incoming request from user:
Ignoring request for user:
-> HTTP 200
It never reached upstream, so nothing was forwarded — which is why it survived.
It is worse in a quieter way: a public endpoint reporting success for a request
it refused to authenticate is exactly what a probe looks for.
The committer name is read from the PAYLOAD, and a payload is only trustworthy
after the signature says so. Deciding anything on it first — including deciding
to IGNORE it — is deciding on attacker-supplied data. Validation now comes
first.
FOUND BY RUNNING IT, NOT BY THE TESTS, and the regression test took two
attempts to get right. The first used the gitlab provider and passed with the
bug deliberately reintroduced, because gitlab has no empty-committer case and
never reaches that branch. The github version fails with "got 200 want 400"
against the old ordering, verified by swapping the blocks back.
BOTH SIGNATURE HEADERS ARE NOW OPTIONAL AT THE PARSER. `GetHeaderKeys` conflated
"headers I want forwarded" with "headers that must be present", so listing
X-Hub-Signature-256 there made BOTH mandatory and rejected every delivery
carrying only one. Requiring either is wrong in a different direction: SHA-256
excludes GitLab and older GitHub Enterprise, SHA-1 excludes a sender that has
dropped the legacy header. The real rule — at least one, strongest wins — can
only be stated in `Validate`, so `GetOptionalHeaderKeys` was added to the
Provider interface and enforcement lives with the signature check.
A SELF-CONTAINED DOCKERFILE. `build/package/Dockerfile` is three lines that
`COPY ./gitwebhookproxy /` — it expects a binary already compiled beside it, so
it cannot be handed to any platform that builds from a repository.
`Dockerfile.build` is a Go 1.13 image driving `packr` and a GOPATH layout
modules replaced. The new root `Dockerfile` builds from source in one pass:
digest-pinned Go 1.25, CGO off, `-trimpath -ldflags="-s -w"`, and a distroless
`static-debian12:nonroot` runtime with no shell and no package manager — upstream
ran on `stakater/base-alpine:3.7`, last updated 2018.
VERIFIED AGAINST THE RUNNING IMAGE, not only in unit tests:
SHA-256 only, valid -> 200
SHA-1 only, valid -> 200
forged SHA-256 + valid 1 -> 400
no signature at all -> 400
tampered body, valid sig -> 400
disallowed path -> 403
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fail cleanly on invalid webhook configuration
feat: gate Jenkins relay bearer on verified webhooks
The upstream bearer was allowed only over HTTPS or literal loopback HTTP, so a relay that is its own service — a public listener must not share a container with the credentials behind it — could never present the bearer to an upstream on Railway's private network, which is plain HTTP by design. That network resolves only inside one project and is WireGuard-encrypted between services: the same boundary as loopback, expressed across two containers. `isRailwayPrivateHost` accepts exactly one lowercase label under `railway.internal`, with no userinfo, query, fragment, trailing dot, nested label or lookalike public suffix; tests cover each rejection and the accepted form. The README's exception list says so. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BbpdUg9nC4zTfjVp8AFSEC
Author
|
Opened against the upstream repository by mistake (the change targets the jbcom fork, which #126 was offered from). Closing; sorry for the noise. |
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
1 participant
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.
The upstream bearer (
GWP_UPSTREAMTOKEN) was allowed only over HTTPS or literal loopback HTTP. A relay deployed as its own Railway service reaches its upstream overhttp://<service>.railway.internal, which is plain HTTP by design and resolves only inside one project's WireGuard-encrypted private network — the same trust boundary as loopback, across two containers. Without this the relay could never present the bearer to Jenkins' Generic Webhook Trigger, which answered 404 ("no jobs for this token") to every signed delivery.isRailwayPrivateHostaccepts exactly one lowercase label underrailway.internal— no userinfo, query, fragment, trailing dot, nested label or lookalike public suffix — with tests for each rejection and the accepted form. README updated.🤖 Generated with Claude Code
https://claude.ai/code/session_01BbpdUg9nC4zTfjVp8AFSEC