Compare commits
2 Commits
3925fce24a
...
0598f1dc04
| Author | SHA1 | Date | |
|---|---|---|---|
| 0598f1dc04 | |||
| 992b3c68f5 |
@@ -19,9 +19,14 @@ RUN go mod download
|
|||||||
# .dockerignore.
|
# .dockerignore.
|
||||||
COPY . .
|
COPY . .
|
||||||
|
|
||||||
# Run formatting check and linter
|
# Run formatting check and linter. golangci-lint is invoked directly rather
|
||||||
|
# than through `make lint`: this stage is already the pinned linter image, and
|
||||||
|
# script/lint is a wrapper that builds Dockerfile.lint, so calling it here
|
||||||
|
# would need a docker daemon inside the build. Keep these steps in step with
|
||||||
|
# Dockerfile.lint, including --network=none (see its header for why).
|
||||||
RUN make fmt-check
|
RUN make fmt-check
|
||||||
RUN make lint
|
RUN --network=none golangci-lint config verify --config .golangci.yml
|
||||||
|
RUN --network=none golangci-lint run --config .golangci.yml ./...
|
||||||
|
|
||||||
# Build stage
|
# Build stage
|
||||||
# golang:1.26.1-bookworm (Debian-based), 2026-03-17
|
# golang:1.26.1-bookworm (Debian-based), 2026-03-17
|
||||||
|
|||||||
37
Dockerfile.lint
Normal file
37
Dockerfile.lint
Normal file
@@ -0,0 +1,37 @@
|
|||||||
|
# Lint-only image, built by script/lint. golangci-lint is never installed on
|
||||||
|
# the host: the repo is COPYed into the pinned image and linted as a build
|
||||||
|
# step, so a successful build IS a clean lint. This works even when the docker
|
||||||
|
# daemon is remote and bind mounts are impossible.
|
||||||
|
#
|
||||||
|
# script/lint passes --no-cache-filter=lint. Without it an unchanged tree
|
||||||
|
# replays the lint stage from cache and the build succeeds in under a second
|
||||||
|
# having run no linter at all. Do not drop that flag.
|
||||||
|
#
|
||||||
|
# The lint steps run with --network=none. `golangci-lint config verify` is
|
||||||
|
# documented as fetching its JSON schema over HTTPS, which would make linting
|
||||||
|
# depend on an unpinned remote artifact; this pinned image resolves the schema
|
||||||
|
# without any network, and --network=none enforces that rather than trusting
|
||||||
|
# it. It also proves no linter reaches out at analysis time. If a future image
|
||||||
|
# bump makes either step need the network, this build fails loudly instead of
|
||||||
|
# quietly acquiring an unpinned dependency.
|
||||||
|
|
||||||
|
# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07
|
||||||
|
# Using Debian-based image because mattn/go-sqlite3 (CGO) does not
|
||||||
|
# compile on Alpine musl (off64_t is a glibc type).
|
||||||
|
FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS deps
|
||||||
|
|
||||||
|
WORKDIR /src
|
||||||
|
|
||||||
|
# Copy go mod files first for better layer caching. This stage is cacheable;
|
||||||
|
# only the lint stage below is forced to re-execute.
|
||||||
|
COPY go.mod go.sum ./
|
||||||
|
RUN go mod download
|
||||||
|
|
||||||
|
FROM deps AS lint
|
||||||
|
|
||||||
|
COPY . .
|
||||||
|
|
||||||
|
# `run` silently ignores config keys it does not recognize, so a typo would
|
||||||
|
# disable a setting without a word. `config verify` is what catches that.
|
||||||
|
RUN --network=none golangci-lint config verify --config .golangci.yml
|
||||||
|
RUN --network=none golangci-lint run --config .golangci.yml ./...
|
||||||
105
README.md
105
README.md
@@ -12,14 +12,16 @@ with retry support, logging, and observability. Category: infrastructure
|
|||||||
### Prerequisites
|
### Prerequisites
|
||||||
|
|
||||||
- Go 1.26.1+ (the version in `go.mod`)
|
- Go 1.26.1+ (the version in `go.mod`)
|
||||||
- golangci-lint v2.12.2 (the version pinned in `script/bootstrap` and
|
- Docker (for linting, for the test stage of the CI gate, and for
|
||||||
in the `Dockerfile`'s lint stage; `make bootstrap` installs it)
|
containerized deployment)
|
||||||
- Docker (for containerized deployment, and for the lint and test
|
|
||||||
stages of the CI gate)
|
|
||||||
- `curl`, used by `script/fetch-assets` to download the third-party
|
- `curl`, used by `script/fetch-assets` to download the third-party
|
||||||
browser assets, which are not committed (`make bootstrap` installs
|
browser assets, which are not committed (`make bootstrap` installs
|
||||||
it if missing)
|
it if missing)
|
||||||
|
|
||||||
|
golangci-lint is not a prerequisite and must not be installed on the
|
||||||
|
host: `script/bootstrap` does not install it, and `make lint` runs the
|
||||||
|
digest-pinned linter image via `Dockerfile.lint`.
|
||||||
|
|
||||||
### Quick Start
|
### Quick Start
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
@@ -27,9 +29,9 @@ with retry support, logging, and observability. Category: infrastructure
|
|||||||
git clone https://git.eeqj.de/sneak/webhooker.git
|
git clone https://git.eeqj.de/sneak/webhooker.git
|
||||||
cd webhooker
|
cd webhooker
|
||||||
|
|
||||||
# Install Go dependencies, the pinned linter, and the third-party
|
# Install Go dependencies and the third-party browser assets.
|
||||||
# browser assets. `make deps` alone is not enough: it only runs
|
# `make deps` alone is not enough: it only runs go mod download/tidy,
|
||||||
# go mod download/tidy, and the checks below need the fetched assets.
|
# and the checks below need the fetched assets.
|
||||||
make bootstrap
|
make bootstrap
|
||||||
|
|
||||||
# Run all checks (test, lint, format check)
|
# Run all checks (test, lint, format check)
|
||||||
@@ -52,7 +54,7 @@ make setup # Bootstrap + install git pre-commit hook
|
|||||||
make assets # Fetch + verify third-party browser assets
|
make assets # Fetch + verify third-party browser assets
|
||||||
make fmt # Format code (gofmt + goimports)
|
make fmt # Format code (gofmt + goimports)
|
||||||
make fmt-check # Fail if gofmt would change anything (writes nothing)
|
make fmt-check # Fail if gofmt would change anything (writes nothing)
|
||||||
make lint # Run golangci-lint
|
make lint # Run golangci-lint in Docker (Dockerfile.lint)
|
||||||
make test # Run tests with race detection
|
make test # Run tests with race detection
|
||||||
make check # test + lint + fmt-check (CI gate)
|
make check # test + lint + fmt-check (CI gate)
|
||||||
make build # Build binary to bin/webhooker
|
make build # Build binary to bin/webhooker
|
||||||
@@ -275,7 +277,7 @@ are inline commands with no script behind them. We provide:
|
|||||||
- `script/fetch-assets` — download the third-party browser assets into
|
- `script/fetch-assets` — download the third-party browser assets into
|
||||||
`static/`, verifying each against its pinned sha256
|
`static/`, verifying each against its pinned sha256
|
||||||
- `script/test` — run the test suite
|
- `script/test` — run the test suite
|
||||||
- `script/lint` — run golangci-lint
|
- `script/lint` — run golangci-lint in Docker (see Linting below)
|
||||||
- `script/fmt` — format all code (writes)
|
- `script/fmt` — format all code (writes)
|
||||||
- `script/fmt-check` — check formatting (read-only)
|
- `script/fmt-check` — check formatting (read-only)
|
||||||
- `script/check` — run test, lint, and fmt-check
|
- `script/check` — run test, lint, and fmt-check
|
||||||
@@ -1020,6 +1022,33 @@ buy the same amplification as an invented path. Nothing debuggable is
|
|||||||
lost: `page`, on the authenticated pagination links, is the only query
|
lost: `page`, on the authenticated pagination links, is the only query
|
||||||
parameter this service reads.
|
parameter this service reads.
|
||||||
|
|
||||||
|
Client-supplied request content does not leave the host by the other
|
||||||
|
route either. The Sentry SDK attaches the request to every event it
|
||||||
|
captures, independently of the access log, and `SendDefaultPII=false`
|
||||||
|
does not cover all of what it copies: the raw query string and the
|
||||||
|
first 10 KiB of the request body are both taken unconditionally, the
|
||||||
|
body precisely because these handlers call `ParseForm`. A `BeforeSend`
|
||||||
|
hook therefore replaces the query string and the body with
|
||||||
|
`(redacted)`, drops cookies and the remote-address environment, and
|
||||||
|
reduces the headers to a fixed allowlist — `Accept`, `Content-Length`,
|
||||||
|
`Content-Type`, `Host`, `Origin`, `Referer`, `User-Agent` and
|
||||||
|
`X-Request-Id`.
|
||||||
|
|
||||||
|
The body is replaced rather than filtered because the hook cannot tell
|
||||||
|
which route it is on: the SDK hands `BeforeSend` no request, so a
|
||||||
|
route-conditional rule would have to guess, and an unrecognised route
|
||||||
|
must not leak. Nothing debuggable is lost by it. Every handler reads
|
||||||
|
its fields with `PostFormValue`, so the body is exactly where the
|
||||||
|
credentials are — the target destination URL, the login password, both
|
||||||
|
password-change fields — and on the receiver route, the one route
|
||||||
|
whose body is genuine signal, that body is already stored on the event
|
||||||
|
and served from the UI. The headers are an allowlist for the same
|
||||||
|
reason: the SDK's own filter removes four names and passes everything
|
||||||
|
else, which would ship `X-CSRF-Token` and the shared secrets senders
|
||||||
|
put on the receiver route. What survives still names the failing
|
||||||
|
route — scheme, host, path, method — and `X-Request-Id` ties the event
|
||||||
|
to the local access log line that holds the rest.
|
||||||
|
|
||||||
The remaining client-supplied fields are truncated rather than dropped,
|
The remaining client-supplied fields are truncated rather than dropped,
|
||||||
each to a fixed budget: 512 bytes for `url`, `useragent` and `referer`,
|
each to a fixed budget: 512 bytes for `url`, `useragent` and `referer`,
|
||||||
128 for `request_id` (chi passes an inbound `X-Request-Id` header
|
128 for `request_id` (chi passes an inbound `X-Request-Id` header
|
||||||
@@ -1247,6 +1276,7 @@ webhooker/
|
|||||||
├── templates/ # Go HTML templates (base, login, sources, etc.)
|
├── templates/ # Go HTML templates (base, login, sources, etc.)
|
||||||
├── script/ # Scripts to Rule Them All entrypoints
|
├── script/ # Scripts to Rule Them All entrypoints
|
||||||
├── Dockerfile # Three stages: lint, test+build, Alpine runtime
|
├── Dockerfile # Three stages: lint, test+build, Alpine runtime
|
||||||
|
├── Dockerfile.lint # Lint-only image built by script/lint
|
||||||
├── Makefile # 10 of 16 targets shim script/; 6 are inline
|
├── Makefile # 10 of 16 targets shim script/; 6 are inline
|
||||||
├── go.mod / go.sum
|
├── go.mod / go.sum
|
||||||
└── .golangci.yml # Linter configuration
|
└── .golangci.yml # Linter configuration
|
||||||
@@ -1452,6 +1482,37 @@ Two operational consequences follow from bounding the sequence:
|
|||||||
no shutdown diagnostics at all. Keep the deployment's grace above
|
no shutdown diagnostics at all. Keep the deployment's grace above
|
||||||
the stop timeout.
|
the stop timeout.
|
||||||
|
|
||||||
|
### Linting
|
||||||
|
|
||||||
|
golangci-lint never runs on the host. `script/lint` builds
|
||||||
|
`Dockerfile.lint`, which copies the repo into the digest-pinned
|
||||||
|
golangci-lint image and lints as a build step, so a successful build is
|
||||||
|
a clean lint. A host binary would share one cache and one lock with
|
||||||
|
every other checkout on the machine, which has produced both invented
|
||||||
|
findings attributed to other worktrees and unearned passes.
|
||||||
|
|
||||||
|
Three properties are load-bearing:
|
||||||
|
|
||||||
|
- `script/lint` passes `--no-cache-filter=lint`. Without it an unchanged
|
||||||
|
tree replays the lint layer from cache and the build exits 0 in under
|
||||||
|
a second having linted nothing. The `deps` stage stays cacheable, so
|
||||||
|
module downloads are not repeated. Invalidation is scoped to the one
|
||||||
|
stage; never prune the shared build cache.
|
||||||
|
- `script/lint` does not trust that flag. Docker silently ignores
|
||||||
|
`--no-cache-filter` for a stage name that does not match, so a stage
|
||||||
|
rename or a one-character typo would restore the cached false green
|
||||||
|
with no warning and a fast exit 0. The script therefore tees the
|
||||||
|
build output and treats a run as a pass only if golangci-lint's own
|
||||||
|
summary line (`N issues.` / `N issues:`) appears in it: no summary,
|
||||||
|
no lint, whatever the exit code says.
|
||||||
|
- Both lint steps use `RUN --network=none`. `golangci-lint config
|
||||||
|
verify` is documented as fetching its JSON schema over HTTPS, which
|
||||||
|
would be an unpinned remote dependency; the pinned image resolves the
|
||||||
|
schema without network access, and `--network=none` enforces that
|
||||||
|
instead of trusting it. Verify is worth keeping because
|
||||||
|
`golangci-lint run` silently ignores config keys it does not
|
||||||
|
recognize, so a typo would disable a setting with no warning.
|
||||||
|
|
||||||
### Docker
|
### Docker
|
||||||
|
|
||||||
The Dockerfile uses a three-stage build. Each stage is pinned by
|
The Dockerfile uses a three-stage build. Each stage is pinned by
|
||||||
@@ -1460,7 +1521,8 @@ version is fixed independently of the compiler's:
|
|||||||
|
|
||||||
1. **Lint stage** (`golangci/golangci-lint:v2.12.2`, Debian-based) —
|
1. **Lint stage** (`golangci/golangci-lint:v2.12.2`, Debian-based) —
|
||||||
installs `make`, downloads dependencies, copies the source, and runs
|
installs `make`, downloads dependencies, copies the source, and runs
|
||||||
`make fmt-check` then `make lint`.
|
`make fmt-check`, then `golangci-lint config verify` and
|
||||||
|
`golangci-lint run`, both with `--network=none`.
|
||||||
2. **Builder stage** (`golang:1.26.1-bookworm`) — depends on the lint
|
2. **Builder stage** (`golang:1.26.1-bookworm`) — depends on the lint
|
||||||
stage passing (it copies a file from it), runs `script/fetch-assets`
|
stage passing (it copies a file from it), runs `script/fetch-assets`
|
||||||
to download and verify the third-party browser assets, then runs
|
to download and verify the third-party browser assets, then runs
|
||||||
@@ -1471,20 +1533,21 @@ version is fixed independently of the compiler's:
|
|||||||
runs as the non-root `webhooker` user (UID 1000), exposes port 8080,
|
runs as the non-root `webhooker` user (UID 1000), exposes port 8080,
|
||||||
and includes a health check against `/.well-known/healthcheck`.
|
and includes a health check against `/.well-known/healthcheck`.
|
||||||
|
|
||||||
|
The lint stage invokes `golangci-lint` directly rather than `make lint`:
|
||||||
|
it is already the pinned linter image, and `make lint` builds
|
||||||
|
`Dockerfile.lint`, which would need a docker daemon inside this build.
|
||||||
|
|
||||||
Both check stages use Debian rather than Alpine because
|
Both check stages use Debian rather than Alpine because
|
||||||
`gorm.io/driver/sqlite` pulls in `mattn/go-sqlite3`, which needs CGO
|
`gorm.io/driver/sqlite` pulls in `mattn/go-sqlite3`, which needs CGO
|
||||||
and does not compile against musl. Only the final binary is statically
|
and does not compile against musl. Only the final binary is statically
|
||||||
linked, which is what lets it run on the Alpine runtime image.
|
linked, which is what lets it run on the Alpine runtime image.
|
||||||
|
|
||||||
`script/cibuild` — `docker build .` — is the CI gate: the four check
|
`script/cibuild` — `docker build .` — is the CI gate: the checks run
|
||||||
targets run inside the image, so a build that succeeds is a repo that
|
inside the image, so a build that succeeds is a repo that is formatted,
|
||||||
is formatted, linted, tested and compiled. Only `script/cibuild` and
|
linted, tested and compiled. `script/lint` also uses Docker
|
||||||
`script/docker` involve Docker. `script/lint`, and therefore
|
(`Dockerfile.lint`, see Linting above), so `make lint` and `make check`
|
||||||
`make lint` and `make check`, run whatever `golangci-lint` is on the
|
run the same pinned linter version the gate does; only `script/test`
|
||||||
host, which can be a different version from the pinned one — so the
|
and `script/fmt-check` run on the host.
|
||||||
container is the authoritative lint result
|
|
||||||
([issue #109](https://git.eeqj.de/sneak/webhooker/issues/109) tracks
|
|
||||||
routing local linting through it as well).
|
|
||||||
|
|
||||||
#### CI gate honesty
|
#### CI gate honesty
|
||||||
|
|
||||||
@@ -1497,8 +1560,8 @@ the hash of the last commit that touched the build context, so:
|
|||||||
- Any commit that changes code (including a squash merge whose tree
|
- Any commit that changes code (including a squash merge whose tree
|
||||||
matches an already-built branch) gets a new fingerprint, invalidates
|
matches an already-built branch) gets a new fingerprint, invalidates
|
||||||
the `COPY . .` layer of both check stages, and really runs
|
the `COPY . .` layer of both check stages, and really runs
|
||||||
`make fmt-check`, `make lint`, `make test`, and `make build`. A run
|
`make fmt-check`, `golangci-lint`, `make test`, and `make build`. A
|
||||||
that reports success ran them.
|
run that reports success ran them.
|
||||||
- A docs-only commit leaves the fingerprint unchanged — `.dockerignore`
|
- A docs-only commit leaves the fingerprint unchanged — `.dockerignore`
|
||||||
excludes `*.md`, `LICENSE` and `.editorconfig` from the context
|
excludes `*.md`, `LICENSE` and `.editorconfig` from the context
|
||||||
anyway — so the image replays from cache and costs seconds.
|
anyway — so the image replays from cache and costs seconds.
|
||||||
|
|||||||
@@ -2,12 +2,16 @@ package database
|
|||||||
|
|
||||||
import "time"
|
import "time"
|
||||||
|
|
||||||
// APIKey represents an API key for a user
|
// APIKey represents an API key for a user.
|
||||||
|
//
|
||||||
|
// Key is a bearer credential, so it is never marshalled with the
|
||||||
|
// model. A creation handler that has to show it once returns it in its
|
||||||
|
// own response type.
|
||||||
type APIKey struct {
|
type APIKey struct {
|
||||||
BaseModel
|
BaseModel
|
||||||
|
|
||||||
UserID string `gorm:"type:uuid;not null" json:"userId"`
|
UserID string `gorm:"type:uuid;not null" json:"userId"`
|
||||||
Key string `gorm:"uniqueIndex;not null" json:"key"`
|
Key string `gorm:"uniqueIndex;not null" json:"-"`
|
||||||
Description string `json:"description"`
|
Description string `json:"description"`
|
||||||
LastUsedAt *time.Time `json:"lastUsedAt,omitempty"`
|
LastUsedAt *time.Time `json:"lastUsedAt,omitempty"`
|
||||||
|
|
||||||
|
|||||||
107
internal/database/model_secrets_test.go
Normal file
107
internal/database/model_secrets_test.go
Normal file
@@ -0,0 +1,107 @@
|
|||||||
|
package database_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"encoding/json"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
|
)
|
||||||
|
|
||||||
|
// keptField is a non-secret value planted alongside each secret, so
|
||||||
|
// the assertions below cannot pass by the model marshalling to nothing.
|
||||||
|
const keptField = "keepme"
|
||||||
|
|
||||||
|
// marshalModel encodes a model the way a future JSON handler would.
|
||||||
|
func marshalModel(t *testing.T, v any) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
encoded, err := json.Marshal(v)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
return string(encoded)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestModelsDoNotMarshalTheirSecrets pins the barrier for the JSON
|
||||||
|
// path. The /api/v1 route group exists and is empty; delivery's
|
||||||
|
// TargetView masks the credential for the HTML path only, so without
|
||||||
|
// these tags the first handler that marshals a model serialises the
|
||||||
|
// secret with it. Each field below is a live credential:
|
||||||
|
//
|
||||||
|
// - Target.Config holds an incoming-webhook URL whose path segments
|
||||||
|
// are the bearer token.
|
||||||
|
// - APIKey.Key is a bearer token outright.
|
||||||
|
// - Setting.Value holds the session encryption key.
|
||||||
|
// - User.Password holds the Argon2 hash, and was already tagged.
|
||||||
|
func TestModelsDoNotMarshalTheirSecrets(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
const marker = "QQMODELMARKERQQ"
|
||||||
|
|
||||||
|
cases := []struct {
|
||||||
|
name string
|
||||||
|
model any
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "target config",
|
||||||
|
model: database.Target{
|
||||||
|
Name: keptField,
|
||||||
|
Type: database.TargetTypeSlack,
|
||||||
|
Config: `{"webhookUrl":"https://h/s/` + marker + `"}`,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "api key",
|
||||||
|
model: database.APIKey{
|
||||||
|
Description: keptField,
|
||||||
|
Key: marker,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "setting value",
|
||||||
|
model: database.Setting{
|
||||||
|
Key: keptField,
|
||||||
|
Value: marker,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "user password hash",
|
||||||
|
model: database.User{
|
||||||
|
Username: keptField,
|
||||||
|
Password: marker,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range cases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
encoded := marshalModel(t, tc.model)
|
||||||
|
|
||||||
|
assert.NotContains(t, encoded, marker)
|
||||||
|
assert.Contains(t, encoded, keptField)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestWebhookMarshalsNoTargetConfig covers the nested case: a webhook
|
||||||
|
// marshalled with its targets preloaded must not carry the credential
|
||||||
|
// through the association either.
|
||||||
|
func TestWebhookMarshalsNoTargetConfig(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
const marker = "QQNESTEDMARKERQQ"
|
||||||
|
|
||||||
|
encoded := marshalModel(t, database.Webhook{
|
||||||
|
Name: keptField,
|
||||||
|
Targets: []database.Target{{
|
||||||
|
Name: "slack",
|
||||||
|
Config: `{"webhookUrl":"https://h/s/` + marker + `"}`,
|
||||||
|
}},
|
||||||
|
})
|
||||||
|
|
||||||
|
assert.NotContains(t, encoded, marker)
|
||||||
|
assert.Contains(t, encoded, keptField)
|
||||||
|
}
|
||||||
@@ -4,5 +4,8 @@ package database
|
|||||||
// Used for auto-generated values like the session encryption key.
|
// Used for auto-generated values like the session encryption key.
|
||||||
type Setting struct {
|
type Setting struct {
|
||||||
Key string `gorm:"primaryKey" json:"key"`
|
Key string `gorm:"primaryKey" json:"key"`
|
||||||
Value string `gorm:"type:text;not null" json:"value"`
|
|
||||||
|
// Value holds the session encryption key, so it is never
|
||||||
|
// marshalled with the model.
|
||||||
|
Value string `gorm:"type:text;not null" json:"-"`
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -20,8 +20,14 @@ type Target struct {
|
|||||||
Type TargetType `gorm:"not null" json:"type"`
|
Type TargetType `gorm:"not null" json:"type"`
|
||||||
Active bool `gorm:"default:true" json:"active"`
|
Active bool `gorm:"default:true" json:"active"`
|
||||||
|
|
||||||
// Configuration fields (JSON stored based on type)
|
// Configuration fields (JSON stored based on type).
|
||||||
Config string `gorm:"type:text" json:"config"` // JSON configuration
|
//
|
||||||
|
// json:"-" because the blob holds the target's credential — a
|
||||||
|
// Slack incoming-webhook URL, or an http destination whose path
|
||||||
|
// segments are the secret. delivery.TargetView is the masking
|
||||||
|
// barrier for the HTML path; this tag is the barrier for any
|
||||||
|
// handler that marshals the model itself.
|
||||||
|
Config string `gorm:"type:text" json:"-"` // JSON configuration
|
||||||
|
|
||||||
// For HTTP targets (max_retries=0 means fire-and-forget,
|
// For HTTP targets (max_retries=0 means fire-and-forget,
|
||||||
// >0 enables retries with backoff)
|
// >0 enables retries with backoff)
|
||||||
|
|||||||
@@ -39,8 +39,10 @@ func (h *Handlers) HandleLoginSubmit() http.HandlerFunc {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
username := r.FormValue("username")
|
// PostFormValue, not FormValue: the credential must come
|
||||||
password := r.FormValue("password")
|
// from the body, never from the query string.
|
||||||
|
username := r.PostFormValue("username")
|
||||||
|
password := r.PostFormValue("password")
|
||||||
|
|
||||||
// Validate input
|
// Validate input
|
||||||
if username == "" || password == "" {
|
if username == "" || password == "" {
|
||||||
|
|||||||
@@ -44,9 +44,11 @@ func (h *Handlers) HandlePasswordChange() http.HandlerFunc {
|
|||||||
successMessage, errorMessage, handled := h.applyPasswordChange(
|
successMessage, errorMessage, handled := h.applyPasswordChange(
|
||||||
w,
|
w,
|
||||||
sessionUsername,
|
sessionUsername,
|
||||||
r.FormValue("current_password"),
|
// PostFormValue, not FormValue: the credential must
|
||||||
r.FormValue("new_password"),
|
// come from the body, never from the query string.
|
||||||
r.FormValue("confirm_password"),
|
r.PostFormValue("current_password"),
|
||||||
|
r.PostFormValue("new_password"),
|
||||||
|
r.PostFormValue("confirm_password"),
|
||||||
)
|
)
|
||||||
if !handled {
|
if !handled {
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -227,9 +227,9 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
name := r.FormValue("name")
|
name := r.PostFormValue("name")
|
||||||
description := r.FormValue("description")
|
description := r.PostFormValue("description")
|
||||||
retentionStr := r.FormValue("retention_days")
|
retentionStr := r.PostFormValue("retention_days")
|
||||||
|
|
||||||
if name == "" {
|
if name == "" {
|
||||||
w.WriteHeader(http.StatusBadRequest)
|
w.WriteHeader(http.StatusBadRequest)
|
||||||
@@ -509,7 +509,7 @@ func (h *Handlers) applyWebhookEdit(
|
|||||||
) {
|
) {
|
||||||
// The body size cap is enforced by the MaxBodySize middleware,
|
// The body size cap is enforced by the MaxBodySize middleware,
|
||||||
// which runs before CSRF parses the form.
|
// which runs before CSRF parses the form.
|
||||||
name := r.FormValue("name")
|
name := r.PostFormValue("name")
|
||||||
if name == "" {
|
if name == "" {
|
||||||
data := map[string]any{
|
data := map[string]any{
|
||||||
tmplKeyWebhook: webhook,
|
tmplKeyWebhook: webhook,
|
||||||
@@ -523,12 +523,12 @@ func (h *Handlers) applyWebhookEdit(
|
|||||||
}
|
}
|
||||||
|
|
||||||
webhook.Name = name
|
webhook.Name = name
|
||||||
webhook.Description = r.FormValue("description")
|
webhook.Description = r.PostFormValue("description")
|
||||||
|
|
||||||
// An empty field falls back to the stored value, so submitting the
|
// An empty field falls back to the stored value, so submitting the
|
||||||
// form without touching retention leaves the policy alone.
|
// form without touching retention leaves the policy alone.
|
||||||
retentionDays, retErr := parseRetentionDays(
|
retentionDays, retErr := parseRetentionDays(
|
||||||
r.FormValue("retention_days"), webhook.RetentionDays,
|
r.PostFormValue("retention_days"), webhook.RetentionDays,
|
||||||
)
|
)
|
||||||
if retErr != nil {
|
if retErr != nil {
|
||||||
data := map[string]any{
|
data := map[string]any{
|
||||||
@@ -950,7 +950,7 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
description := r.FormValue("description")
|
description := r.PostFormValue("description")
|
||||||
|
|
||||||
entrypoint := &database.Entrypoint{
|
entrypoint := &database.Entrypoint{
|
||||||
WebhookID: webhook.ID,
|
WebhookID: webhook.ID,
|
||||||
@@ -1020,11 +1020,18 @@ func (h *Handlers) processTargetCreate(
|
|||||||
) {
|
) {
|
||||||
// The body size cap is enforced by the MaxBodySize middleware,
|
// The body size cap is enforced by the MaxBodySize middleware,
|
||||||
// which runs before CSRF parses the form.
|
// which runs before CSRF parses the form.
|
||||||
name := r.FormValue("name")
|
//
|
||||||
targetType := database.TargetType(r.FormValue("type"))
|
// Every field here is read with PostFormValue, not FormValue.
|
||||||
targetURL := r.FormValue("url")
|
// FormValue falls back to the query string, which would let
|
||||||
maxRetriesStr := r.FormValue("max_retries")
|
// `POST /source/{id}/targets?url=https://hooks.slack.com/...`
|
||||||
expiry := r.FormValue("expiry")
|
// configure a target from a value the request line carries — and
|
||||||
|
// the request line, unlike the body, is what logs, proxies,
|
||||||
|
// Referer headers and error trackers record.
|
||||||
|
name := r.PostFormValue("name")
|
||||||
|
targetType := database.TargetType(r.PostFormValue("type"))
|
||||||
|
targetURL := r.PostFormValue("url")
|
||||||
|
maxRetriesStr := r.PostFormValue("max_retries")
|
||||||
|
expiry := r.PostFormValue("expiry")
|
||||||
|
|
||||||
if name == "" {
|
if name == "" {
|
||||||
http.Error(
|
http.Error(
|
||||||
|
|||||||
206
internal/handlers/target_create_query_test.go
Normal file
206
internal/handlers/target_create_query_test.go
Normal file
@@ -0,0 +1,206 @@
|
|||||||
|
package handlers_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"context"
|
||||||
|
"log/slog"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"net/url"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/go-chi/chi"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"sneak.berlin/go/webhooker/internal/config"
|
||||||
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
|
"sneak.berlin/go/webhooker/internal/middleware"
|
||||||
|
)
|
||||||
|
|
||||||
|
// targetSecretSegments are the path segments of an incoming-webhook
|
||||||
|
// URL. For Slack, Discord and Teams the path IS the bearer credential,
|
||||||
|
// so this string must not reach storage or the access log by way of
|
||||||
|
// the request line.
|
||||||
|
const targetSecretSegments = "T00000000/B00000000/QQTARGETSECRETQQ"
|
||||||
|
|
||||||
|
// targetSecretURL is a destination whose secret lives in its path. It
|
||||||
|
// uses a literal public address rather than a hostname so the SSRF
|
||||||
|
// check resolves nothing: with a hostname, a sandbox without DNS would
|
||||||
|
// reject the URL for the wrong reason and the test would pass even
|
||||||
|
// with the defect reintroduced.
|
||||||
|
const targetSecretURL = "https://93.184.216.34/services/" +
|
||||||
|
targetSecretSegments
|
||||||
|
|
||||||
|
// targetsForWebhook returns every target stored against a webhook.
|
||||||
|
func targetsForWebhook(
|
||||||
|
t *testing.T,
|
||||||
|
db *database.Database,
|
||||||
|
webhookID string,
|
||||||
|
) []database.Target {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
var targets []database.Target
|
||||||
|
|
||||||
|
require.NoError(
|
||||||
|
t,
|
||||||
|
db.DB().Where("webhook_id = ?", webhookID).
|
||||||
|
Find(&targets).Error,
|
||||||
|
)
|
||||||
|
|
||||||
|
return targets
|
||||||
|
}
|
||||||
|
|
||||||
|
// postTargetCreate drives HandleTargetCreate through the production
|
||||||
|
// access-log middleware and a chi route, so the logged url field is
|
||||||
|
// produced exactly as it ships, and returns the recorder plus the
|
||||||
|
// captured log.
|
||||||
|
func postTargetCreate(
|
||||||
|
t *testing.T,
|
||||||
|
env *sourceTestEnv,
|
||||||
|
webhookID string,
|
||||||
|
query string,
|
||||||
|
form url.Values,
|
||||||
|
) (*httptest.ResponseRecorder, string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
logBuf := new(bytes.Buffer)
|
||||||
|
mw := middleware.NewForTest(
|
||||||
|
slog.New(slog.NewJSONHandler(
|
||||||
|
logBuf, &slog.HandlerOptions{Level: slog.LevelInfo},
|
||||||
|
)),
|
||||||
|
&config.Config{Environment: config.EnvironmentDev},
|
||||||
|
nil,
|
||||||
|
)
|
||||||
|
|
||||||
|
router := chi.NewRouter()
|
||||||
|
router.Use(mw.Logging())
|
||||||
|
router.Post(
|
||||||
|
"/source/{sourceID}/targets",
|
||||||
|
env.handlers.HandleTargetCreate(),
|
||||||
|
)
|
||||||
|
|
||||||
|
target := "/source/" + webhookID + "/targets"
|
||||||
|
if query != "" {
|
||||||
|
target += "?" + query
|
||||||
|
}
|
||||||
|
|
||||||
|
body := ""
|
||||||
|
if form != nil {
|
||||||
|
body = form.Encode()
|
||||||
|
}
|
||||||
|
|
||||||
|
req := httptest.NewRequestWithContext(
|
||||||
|
context.Background(),
|
||||||
|
http.MethodPost,
|
||||||
|
target,
|
||||||
|
strings.NewReader(body),
|
||||||
|
)
|
||||||
|
req.Header.Set(
|
||||||
|
"Content-Type", "application/x-www-form-urlencoded",
|
||||||
|
)
|
||||||
|
|
||||||
|
for _, c := range env.cookies {
|
||||||
|
req.AddCookie(c)
|
||||||
|
}
|
||||||
|
|
||||||
|
w := httptest.NewRecorder()
|
||||||
|
router.ServeHTTP(w, req)
|
||||||
|
|
||||||
|
return w, logBuf.String()
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleTargetCreate_QueryStringURLDoesNotConfigureATarget is the
|
||||||
|
// regression test for the ingress leak. r.FormValue falls back to the
|
||||||
|
// query string when a field is absent from the POST body, so
|
||||||
|
//
|
||||||
|
// POST /source/{id}/targets?url=https://hooks.slack.com/services/...
|
||||||
|
//
|
||||||
|
// with an empty url field used to create a working target from a value
|
||||||
|
// carried on the request line — where logs, proxies, Referer headers
|
||||||
|
// and error trackers record it. The handler reads the body only, so
|
||||||
|
// the request is rejected for a missing URL and stores nothing.
|
||||||
|
//
|
||||||
|
// name and type are sent in the BODY on purpose: the request has to
|
||||||
|
// get past those two validations for the assertion to be about the url
|
||||||
|
// read specifically.
|
||||||
|
func TestHandleTargetCreate_QueryStringURLDoesNotConfigureATarget(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
env := setupSourceTest(t)
|
||||||
|
webhook := seedWebhookWithRetention(t, env.db, 30)
|
||||||
|
|
||||||
|
body := url.Values{}
|
||||||
|
body.Set("name", "leaky")
|
||||||
|
body.Set("type", string(database.TargetTypeSlack))
|
||||||
|
|
||||||
|
w, logged := postTargetCreate(
|
||||||
|
t, env, webhook.ID,
|
||||||
|
"url="+url.QueryEscape(targetSecretURL),
|
||||||
|
body,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert.Equal(t, http.StatusBadRequest, w.Code)
|
||||||
|
|
||||||
|
targets := targetsForWebhook(t, env.db, webhook.ID)
|
||||||
|
assert.Empty(
|
||||||
|
t, targets,
|
||||||
|
"a query-string value must not populate a target config",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert.NotContains(t, logged, targetSecretSegments)
|
||||||
|
assert.NotContains(t, logged, "93.184.216.34")
|
||||||
|
assert.NotEmpty(t, logged, "the access log line must still be written")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleTargetCreate_BodyURLStillCreatesTheTarget is the positive
|
||||||
|
// control for the test above: the rejection has to come from where the
|
||||||
|
// value was read, not from the handler being broken.
|
||||||
|
func TestHandleTargetCreate_BodyURLStillCreatesTheTarget(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
env := setupSourceTest(t)
|
||||||
|
webhook := seedWebhookWithRetention(t, env.db, 30)
|
||||||
|
|
||||||
|
body := url.Values{}
|
||||||
|
body.Set("name", "legit")
|
||||||
|
body.Set("type", string(database.TargetTypeSlack))
|
||||||
|
body.Set("url", targetSecretURL)
|
||||||
|
|
||||||
|
w, logged := postTargetCreate(t, env, webhook.ID, "", body)
|
||||||
|
|
||||||
|
assert.Equal(t, http.StatusSeeOther, w.Code)
|
||||||
|
|
||||||
|
targets := targetsForWebhook(t, env.db, webhook.ID)
|
||||||
|
require.Len(t, targets, 1)
|
||||||
|
assert.Contains(t, targets[0].Config, targetSecretSegments)
|
||||||
|
|
||||||
|
// The body carried the credential, so the access log must still
|
||||||
|
// not have it: the log records the request line only.
|
||||||
|
assert.NotContains(t, logged, targetSecretSegments)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleTargetCreate_QueryStringCannotSupplyNameOrType covers the
|
||||||
|
// rest of the converted reads on this handler in one request: with an
|
||||||
|
// empty body, nothing the query carries is visible to it.
|
||||||
|
func TestHandleTargetCreate_QueryStringCannotSupplyNameOrType(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
env := setupSourceTest(t)
|
||||||
|
webhook := seedWebhookWithRetention(t, env.db, 30)
|
||||||
|
|
||||||
|
w, _ := postTargetCreate(
|
||||||
|
t, env, webhook.ID,
|
||||||
|
"name=leaky&type=slack&max_retries=9&expiry=30d&url="+
|
||||||
|
url.QueryEscape(targetSecretURL),
|
||||||
|
url.Values{},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert.Equal(t, http.StatusBadRequest, w.Code)
|
||||||
|
assert.Contains(t, w.Body.String(), "Name is required")
|
||||||
|
assert.Empty(t, targetsForWebhook(t, env.db, webhook.ID))
|
||||||
|
}
|
||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"log/slog"
|
"log/slog"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
|
||||||
|
"github.com/getsentry/sentry-go"
|
||||||
"sneak.berlin/go/webhooker/internal/config"
|
"sneak.berlin/go/webhooker/internal/config"
|
||||||
"sneak.berlin/go/webhooker/internal/handlers"
|
"sneak.berlin/go/webhooker/internal/handlers"
|
||||||
"sneak.berlin/go/webhooker/internal/middleware"
|
"sneak.berlin/go/webhooker/internal/middleware"
|
||||||
@@ -13,6 +14,25 @@ import (
|
|||||||
// build requests that sit exactly at, below, and above it.
|
// build requests that sit exactly at, below, and above it.
|
||||||
const MaxFormBodySizeForTest = maxFormBodySize
|
const MaxFormBodySizeForTest = maxFormBodySize
|
||||||
|
|
||||||
|
// ScrubSentryRequestForTest exposes the BeforeSend hook that
|
||||||
|
// enableSentry installs, so a test can assert on what it leaves in an
|
||||||
|
// event without standing up a Sentry client.
|
||||||
|
func ScrubSentryRequestForTest(
|
||||||
|
event *sentry.Event,
|
||||||
|
hint *sentry.EventHint,
|
||||||
|
) *sentry.Event {
|
||||||
|
return scrubSentryRequest(event, hint)
|
||||||
|
}
|
||||||
|
|
||||||
|
// SentryClientOptionsForTest exposes the exact options enableSentry
|
||||||
|
// initialises the SDK with, so a test can capture events through the
|
||||||
|
// production hook wiring rather than a hand-built equivalent.
|
||||||
|
func SentryClientOptionsForTest(
|
||||||
|
dsn, release string,
|
||||||
|
) sentry.ClientOptions {
|
||||||
|
return sentryClientOptions(dsn, release)
|
||||||
|
}
|
||||||
|
|
||||||
// NewRouterForTest builds the real route tree via SetupRoutes with
|
// NewRouterForTest builds the real route tree via SetupRoutes with
|
||||||
// the supplied middleware and handlers, bypassing the fx lifecycle
|
// the supplied middleware and handlers, bypassing the fx lifecycle
|
||||||
// and the HTTP listener. Tests use it so that route-group middleware
|
// and the HTTP listener. Tests use it so that route-group middleware
|
||||||
|
|||||||
117
internal/server/sentry.go
Normal file
117
internal/server/sentry.go
Normal file
@@ -0,0 +1,117 @@
|
|||||||
|
package server
|
||||||
|
|
||||||
|
import (
|
||||||
|
"net/http"
|
||||||
|
|
||||||
|
"github.com/getsentry/sentry-go"
|
||||||
|
)
|
||||||
|
|
||||||
|
// sentryRedacted stands in for a withheld field on every event shipped
|
||||||
|
// to Sentry. It is a marker rather than an empty string so a reader
|
||||||
|
// can tell a suppressed value from an absent one.
|
||||||
|
const sentryRedacted = "(redacted)"
|
||||||
|
|
||||||
|
// sentryClientOptions builds the options the SDK is initialised with.
|
||||||
|
// It is its own function so a test can stand up a client wired exactly
|
||||||
|
// as production is, with only the transport swapped.
|
||||||
|
func sentryClientOptions(dsn, release string) sentry.ClientOptions {
|
||||||
|
return sentry.ClientOptions{
|
||||||
|
Dsn: dsn,
|
||||||
|
Release: release,
|
||||||
|
// Both hooks, because the SDK runs one for error events
|
||||||
|
// and the other for transactions.
|
||||||
|
BeforeSend: scrubSentryRequest,
|
||||||
|
BeforeSendTransaction: scrubSentryRequest,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// scrubSentryRequest strips client-supplied content from an event's
|
||||||
|
// request context before it leaves the process.
|
||||||
|
//
|
||||||
|
// sentryhttp attaches the whole *http.Request to the scope
|
||||||
|
// (sentryhttp.go:113), and Scope.ApplyToEvent fills the event's
|
||||||
|
// Request from it inside prepareEvent, which runs before this hook.
|
||||||
|
// Two of the fields it fills are copied with no SendDefaultPII guard:
|
||||||
|
//
|
||||||
|
// - QueryString, verbatim from r.URL.RawQuery.
|
||||||
|
// - Data, the first 10 KiB of the request body, teed off r.Body by
|
||||||
|
// SetRequest and filled precisely because the handlers call
|
||||||
|
// ParseForm.
|
||||||
|
//
|
||||||
|
// Since every form field in this service is read with PostFormValue,
|
||||||
|
// the body is the only place a credential is submitted: a target's
|
||||||
|
// destination URL, whose path segments are the bearer token, plus the
|
||||||
|
// login password and both password-change fields. None of that may
|
||||||
|
// reach a third-party service.
|
||||||
|
//
|
||||||
|
// This hook is a floor, not a default: the fields it clears stay
|
||||||
|
// cleared even if SendDefaultPII is ever turned on.
|
||||||
|
func scrubSentryRequest(
|
||||||
|
event *sentry.Event,
|
||||||
|
_ *sentry.EventHint,
|
||||||
|
) *sentry.Event {
|
||||||
|
if event == nil || event.Request == nil {
|
||||||
|
return event
|
||||||
|
}
|
||||||
|
|
||||||
|
req := event.Request
|
||||||
|
|
||||||
|
if req.QueryString != "" {
|
||||||
|
req.QueryString = sentryRedacted
|
||||||
|
}
|
||||||
|
|
||||||
|
if req.Data != "" {
|
||||||
|
req.Data = sentryRedacted
|
||||||
|
}
|
||||||
|
|
||||||
|
req.Cookies = ""
|
||||||
|
req.Env = nil
|
||||||
|
req.Headers = keptSentryHeaders(req.Headers)
|
||||||
|
|
||||||
|
return event
|
||||||
|
}
|
||||||
|
|
||||||
|
// keptSentryHeaders returns the subset of headers an event may carry
|
||||||
|
// off-host. Dropping by allowlist rather than by blocklist is what
|
||||||
|
// makes an unrecognised header safe: the SDK's own filter removes four
|
||||||
|
// names and passes everything else, so X-Csrf-Token — which
|
||||||
|
// gorilla/csrf accepts in place of the form field — and the shared
|
||||||
|
// secrets senders put on the receiver route (X-Gitlab-Token and the
|
||||||
|
// per-provider signature headers) would otherwise ship verbatim.
|
||||||
|
func keptSentryHeaders(headers map[string]string) map[string]string {
|
||||||
|
if len(headers) == 0 {
|
||||||
|
return headers
|
||||||
|
}
|
||||||
|
|
||||||
|
kept := make(map[string]string, len(headers))
|
||||||
|
|
||||||
|
for name, value := range headers {
|
||||||
|
if sentryKeepsHeader(name) {
|
||||||
|
kept[name] = value
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return kept
|
||||||
|
}
|
||||||
|
|
||||||
|
// sentryKeepsHeader reports whether a request header is routing or
|
||||||
|
// content metadata rather than client-chosen payload. Referer is kept
|
||||||
|
// on the reasoning that it is browser-set, that this service emits
|
||||||
|
// only ?page= in its own links, and that Referrer-Policy is set to
|
||||||
|
// strict-origin-when-cross-origin. X-Request-Id ties the event to the
|
||||||
|
// local access log line, which holds the rest of the detail.
|
||||||
|
func sentryKeepsHeader(name string) bool {
|
||||||
|
switch http.CanonicalHeaderKey(name) {
|
||||||
|
case "Accept",
|
||||||
|
"Content-Length",
|
||||||
|
"Content-Type",
|
||||||
|
"Host",
|
||||||
|
"Origin",
|
||||||
|
"Referer",
|
||||||
|
"User-Agent",
|
||||||
|
"X-Request-Id":
|
||||||
|
return true
|
||||||
|
default:
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
227
internal/server/sentry_test.go
Normal file
227
internal/server/sentry_test.go
Normal file
@@ -0,0 +1,227 @@
|
|||||||
|
package server_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"encoding/json"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"net/url"
|
||||||
|
"strings"
|
||||||
|
"sync"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/getsentry/sentry-go"
|
||||||
|
sentryhttp "github.com/getsentry/sentry-go/http"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"sneak.berlin/go/webhooker/internal/server"
|
||||||
|
)
|
||||||
|
|
||||||
|
// The three markers below are the credentials a captured event could
|
||||||
|
// carry off-host, one per field of sentry.Request that the SDK fills
|
||||||
|
// from the request without a SendDefaultPII guard.
|
||||||
|
const (
|
||||||
|
// sentryBodyMarker is submitted as a form value. Since every
|
||||||
|
// handler reads its fields with PostFormValue, the body is the
|
||||||
|
// only place a password or a target URL is ever supplied.
|
||||||
|
sentryBodyMarker = "QQSENTRYBODYMARKERQQ"
|
||||||
|
|
||||||
|
// sentryQueryMarker rides the request line.
|
||||||
|
sentryQueryMarker = "T00000000/B00000000/QQSENTRYQUERYMARKERQQ"
|
||||||
|
|
||||||
|
// sentryHeaderMarker rides X-Csrf-Token, which gorilla/csrf
|
||||||
|
// accepts in place of the form field.
|
||||||
|
sentryHeaderMarker = "QQSENTRYHEADERMARKERQQ"
|
||||||
|
)
|
||||||
|
|
||||||
|
// sentryKeptUserAgent is a non-secret header value planted so the
|
||||||
|
// assertions below cannot pass by the event carrying no headers at
|
||||||
|
// all.
|
||||||
|
const sentryKeptUserAgent = "webhooker-test-agent"
|
||||||
|
|
||||||
|
// captureTransport records events instead of shipping them, so a test
|
||||||
|
// sees exactly the payload the SDK would have put on the wire.
|
||||||
|
type captureTransport struct {
|
||||||
|
mu sync.Mutex
|
||||||
|
events []*sentry.Event
|
||||||
|
}
|
||||||
|
|
||||||
|
func (c *captureTransport) Configure(sentry.ClientOptions) {}
|
||||||
|
|
||||||
|
func (c *captureTransport) Flush(time.Duration) bool { return true }
|
||||||
|
|
||||||
|
func (c *captureTransport) SendEvent(event *sentry.Event) {
|
||||||
|
c.mu.Lock()
|
||||||
|
defer c.mu.Unlock()
|
||||||
|
|
||||||
|
c.events = append(c.events, event)
|
||||||
|
}
|
||||||
|
|
||||||
|
// captureThroughSentryHTTP panics inside a form handler wrapped in the
|
||||||
|
// real sentryhttp middleware and returns the event the SDK produced.
|
||||||
|
//
|
||||||
|
// This is the only construction path on which Request.Data appears:
|
||||||
|
// sentryhttp calls Scope.SetRequest, which tees r.Body into a 10 KiB
|
||||||
|
// buffer, ParseForm drains the tee, and Scope.ApplyToEvent copies the
|
||||||
|
// buffer into the event inside prepareEvent — before BeforeSend runs.
|
||||||
|
// A hand-built sentry.NewRequest never reads the body and so cannot
|
||||||
|
// regress-test any of it.
|
||||||
|
//
|
||||||
|
// scrub selects whether the production BeforeSend hooks are installed,
|
||||||
|
// so the same path shows both what the SDK collects and what survives.
|
||||||
|
func captureThroughSentryHTTP(t *testing.T, scrub bool) *sentry.Event {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
transport := &captureTransport{}
|
||||||
|
|
||||||
|
opts := server.SentryClientOptionsForTest(
|
||||||
|
"https://public@sentry.invalid/1", "webhooker-test",
|
||||||
|
)
|
||||||
|
opts.Transport = transport
|
||||||
|
|
||||||
|
if !scrub {
|
||||||
|
opts.BeforeSend = nil
|
||||||
|
opts.BeforeSendTransaction = nil
|
||||||
|
}
|
||||||
|
|
||||||
|
client, err := sentry.NewClient(opts)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
handler := sentryhttp.New(sentryhttp.Options{}).Handle(
|
||||||
|
http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
|
||||||
|
// This call is what drains the tee and fills the
|
||||||
|
// buffer. Its success is asserted by the unscrubbed
|
||||||
|
// case below, which sees the body in the event.
|
||||||
|
_ = r.ParseForm()
|
||||||
|
|
||||||
|
panic("boom")
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
|
||||||
|
handler.ServeHTTP(
|
||||||
|
httptest.NewRecorder(),
|
||||||
|
sentryLoginRequest(client),
|
||||||
|
)
|
||||||
|
|
||||||
|
require.Len(t, transport.events, 1)
|
||||||
|
|
||||||
|
return transport.events[0]
|
||||||
|
}
|
||||||
|
|
||||||
|
// sentryLoginRequest builds the password POST the capture above drives,
|
||||||
|
// with a credential planted in the body, the query and a header.
|
||||||
|
func sentryLoginRequest(client *sentry.Client) *http.Request {
|
||||||
|
form := url.Values{}
|
||||||
|
form.Set("username", "admin")
|
||||||
|
form.Set("password", sentryBodyMarker)
|
||||||
|
|
||||||
|
req := httptest.NewRequestWithContext(
|
||||||
|
sentry.SetHubOnContext(
|
||||||
|
context.Background(),
|
||||||
|
sentry.NewHub(client, sentry.NewScope()),
|
||||||
|
),
|
||||||
|
http.MethodPost,
|
||||||
|
"/pages/login?url=https://hooks.slack.com/services/"+
|
||||||
|
sentryQueryMarker,
|
||||||
|
strings.NewReader(form.Encode()),
|
||||||
|
)
|
||||||
|
|
||||||
|
req.Header.Set(
|
||||||
|
"Content-Type", "application/x-www-form-urlencoded",
|
||||||
|
)
|
||||||
|
req.Header.Set("X-Csrf-Token", sentryHeaderMarker)
|
||||||
|
req.Header.Set("User-Agent", sentryKeptUserAgent)
|
||||||
|
|
||||||
|
return req
|
||||||
|
}
|
||||||
|
|
||||||
|
// marshalEvent encodes an event the way the transport does.
|
||||||
|
func marshalEvent(t *testing.T, event *sentry.Event) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
encoded, err := json.Marshal(event)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
return string(encoded)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestSentryScrub_SDKCollectsTheRequestUnscrubbed pins the premise the
|
||||||
|
// hook exists for. Without it the SDK ships the whole POST body, the
|
||||||
|
// raw query and the CSRF header, none of which SendDefaultPII=false
|
||||||
|
// suppresses.
|
||||||
|
func TestSentryScrub_SDKCollectsTheRequestUnscrubbed(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
event := captureThroughSentryHTTP(t, false)
|
||||||
|
require.NotNil(t, event.Request)
|
||||||
|
|
||||||
|
assert.Contains(
|
||||||
|
t, event.Request.Data, sentryBodyMarker,
|
||||||
|
"the SDK is expected to collect the POST body; if it no "+
|
||||||
|
"longer does, the scrub hook's premise changed",
|
||||||
|
)
|
||||||
|
assert.Contains(t, event.Request.QueryString, sentryQueryMarker)
|
||||||
|
assert.Contains(
|
||||||
|
t, marshalEvent(t, event), sentryHeaderMarker,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestSentryScrub_RedactsTheCapturedRequest is the regression test: no
|
||||||
|
// byte of any planted credential may survive into the marshalled event
|
||||||
|
// that leaves the process.
|
||||||
|
func TestSentryScrub_RedactsTheCapturedRequest(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
event := captureThroughSentryHTTP(t, true)
|
||||||
|
require.NotNil(t, event.Request)
|
||||||
|
|
||||||
|
encoded := marshalEvent(t, event)
|
||||||
|
|
||||||
|
assert.NotContains(t, encoded, sentryBodyMarker)
|
||||||
|
assert.NotContains(t, encoded, sentryQueryMarker)
|
||||||
|
assert.NotContains(t, encoded, sentryHeaderMarker)
|
||||||
|
assert.NotContains(t, encoded, "hooks.slack.com")
|
||||||
|
|
||||||
|
assert.Equal(t, "(redacted)", event.Request.Data)
|
||||||
|
assert.Equal(t, "(redacted)", event.Request.QueryString)
|
||||||
|
assert.Empty(t, event.Request.Cookies)
|
||||||
|
assert.Empty(t, event.Request.Env)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestSentryScrub_KeepsTheRoutingContext checks the hook does not cost
|
||||||
|
// the debugging signal: the route, the method and the metadata headers
|
||||||
|
// still identify what failed.
|
||||||
|
func TestSentryScrub_KeepsTheRoutingContext(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
event := captureThroughSentryHTTP(t, true)
|
||||||
|
require.NotNil(t, event.Request)
|
||||||
|
|
||||||
|
assert.Contains(t, event.Request.URL, "/pages/login")
|
||||||
|
assert.Equal(t, http.MethodPost, event.Request.Method)
|
||||||
|
assert.Equal(
|
||||||
|
t,
|
||||||
|
sentryKeptUserAgent,
|
||||||
|
event.Request.Headers["User-Agent"],
|
||||||
|
)
|
||||||
|
assert.Equal(
|
||||||
|
t,
|
||||||
|
"application/x-www-form-urlencoded",
|
||||||
|
event.Request.Headers["Content-Type"],
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestSentryScrub_ToleratesEventsWithoutARequest covers the events the
|
||||||
|
// hook sees outside an HTTP handler, where no request is attached.
|
||||||
|
func TestSentryScrub_ToleratesEventsWithoutARequest(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
scrubbed := server.ScrubSentryRequestForTest(
|
||||||
|
sentry.NewEvent(), nil,
|
||||||
|
)
|
||||||
|
|
||||||
|
require.NotNil(t, scrubbed)
|
||||||
|
assert.Nil(t, scrubbed.Request)
|
||||||
|
assert.Nil(t, server.ScrubSentryRequestForTest(nil, nil))
|
||||||
|
}
|
||||||
@@ -141,14 +141,14 @@ func (s *Server) enableSentry() {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
err := sentry.Init(sentry.ClientOptions{
|
err := sentry.Init(sentryClientOptions(
|
||||||
Dsn: s.params.Config.SentryDSN,
|
s.params.Config.SentryDSN,
|
||||||
Release: fmt.Sprintf(
|
fmt.Sprintf(
|
||||||
"%s-%s",
|
"%s-%s",
|
||||||
s.params.Globals.Appname,
|
s.params.Globals.Appname,
|
||||||
s.params.Globals.Version,
|
s.params.Globals.Version,
|
||||||
),
|
),
|
||||||
})
|
))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
s.log.Error("sentry init failure", "error", err)
|
s.log.Error("sentry init failure", "error", err)
|
||||||
// Don't use fatal since we still want the service to run
|
// Don't use fatal since we still want the service to run
|
||||||
|
|||||||
@@ -3,20 +3,14 @@
|
|||||||
# this repo. Idempotent: every install is guarded by a check so already
|
# this repo. Idempotent: every install is guarded by a check so already
|
||||||
# installed tools are skipped. Base tooling comes from nix, apt, brew,
|
# installed tools are skipped. Base tooling comes from nix, apt, brew,
|
||||||
# or apk (detected in that order); assumes NOTHING is present (not git,
|
# or apk (detected in that order); assumes NOTHING is present (not git,
|
||||||
# make, or go). golangci-lint is packaged in nix, brew, and apk; on apt
|
# make, or go). golangci-lint is deliberately not installed: linting runs
|
||||||
# it is installed from a hash-verified GitHub release archive (never
|
# only in docker, via script/lint and Dockerfile.lint. Finishes by running
|
||||||
# curl | sh). Finishes by running script/fetch-assets, which installs the
|
# script/fetch-assets, which installs the hash-pinned third-party browser
|
||||||
# hash-pinned third-party browser assets the repo does not commit.
|
# assets the repo does not commit.
|
||||||
set -eu
|
set -eu
|
||||||
|
|
||||||
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
||||||
|
|
||||||
# Pinned versions, 2026-08-07. Never "latest"; exact versions only.
|
|
||||||
GOLANGCI_LINT_VERSION="2.12.2"
|
|
||||||
# sha256 of golangci-lint-2.12.2-linux-<arch>.tar.gz release archives
|
|
||||||
GOLANGCI_LINT_SHA256_AMD64="8df580d2670fed8fa984aac0507099af8df275e665215f5c7a2ae3943893a553"
|
|
||||||
GOLANGCI_LINT_SHA256_ARM64="44cd40a8c76c86755375adfeea52cfd3533cb43d7bd647771e0ae065e166df3a"
|
|
||||||
|
|
||||||
PKGMGR=""
|
PKGMGR=""
|
||||||
SUDO=""
|
SUDO=""
|
||||||
|
|
||||||
@@ -57,52 +51,6 @@ missing() {
|
|||||||
! command -v "$1" >/dev/null 2>&1
|
! command -v "$1" >/dev/null 2>&1
|
||||||
}
|
}
|
||||||
|
|
||||||
# verify_sha256 <file> <expected-hash>
|
|
||||||
verify_sha256() {
|
|
||||||
if command -v sha256sum >/dev/null 2>&1; then
|
|
||||||
actual="$(sha256sum "$1" | cut -d' ' -f1)"
|
|
||||||
else
|
|
||||||
actual="$(shasum -a 256 "$1" | cut -d' ' -f1)"
|
|
||||||
fi
|
|
||||||
if [ "$actual" != "$2" ]; then
|
|
||||||
echo "bootstrap: sha256 mismatch for $1" >&2
|
|
||||||
echo " expected: $2" >&2
|
|
||||||
echo " actual: $actual" >&2
|
|
||||||
exit 1
|
|
||||||
fi
|
|
||||||
}
|
|
||||||
|
|
||||||
# apt has no golangci-lint package: install a pinned release archive
|
|
||||||
# from GitHub, verified by hardcoded sha256 (never curl | sh).
|
|
||||||
install_golangci_lint_release() {
|
|
||||||
case "$(uname -m)" in
|
|
||||||
x86_64) goarch="amd64"; sha="$GOLANGCI_LINT_SHA256_AMD64" ;;
|
|
||||||
aarch64|arm64) goarch="arm64"; sha="$GOLANGCI_LINT_SHA256_ARM64" ;;
|
|
||||||
*)
|
|
||||||
echo "bootstrap: unsupported architecture $(uname -m)" >&2
|
|
||||||
exit 1
|
|
||||||
;;
|
|
||||||
esac
|
|
||||||
if missing curl; then pkg_install curl curl curl curl; fi
|
|
||||||
name="golangci-lint-${GOLANGCI_LINT_VERSION}-linux-${goarch}"
|
|
||||||
tmp="$(mktemp -d)"
|
|
||||||
curl -fsSL -o "$tmp/$name.tar.gz" \
|
|
||||||
"https://github.com/golangci/golangci-lint/releases/download/v${GOLANGCI_LINT_VERSION}/${name}.tar.gz"
|
|
||||||
verify_sha256 "$tmp/$name.tar.gz" "$sha"
|
|
||||||
tar -xzf "$tmp/$name.tar.gz" -C "$tmp"
|
|
||||||
$SUDO install -m 0755 "$tmp/$name/golangci-lint" /usr/local/bin/golangci-lint
|
|
||||||
rm -rf "$tmp"
|
|
||||||
}
|
|
||||||
|
|
||||||
ensure_golangci_lint() {
|
|
||||||
if ! missing golangci-lint; then return 0; fi
|
|
||||||
detect_pkgmgr
|
|
||||||
case "$PKGMGR" in
|
|
||||||
apt) install_golangci_lint_release ;;
|
|
||||||
*) pkg_install golangci-lint golangci-lint golangci-lint golangci-lint ;;
|
|
||||||
esac
|
|
||||||
}
|
|
||||||
|
|
||||||
main() {
|
main() {
|
||||||
cd "$ROOT"
|
cd "$ROOT"
|
||||||
|
|
||||||
@@ -110,9 +58,14 @@ main() {
|
|||||||
if missing git; then pkg_install git git git git; fi
|
if missing git; then pkg_install git git git git; fi
|
||||||
if missing make; then pkg_install gnumake make make make; fi
|
if missing make; then pkg_install gnumake make make make; fi
|
||||||
|
|
||||||
# Go toolchain and linter
|
# Go toolchain
|
||||||
if missing go; then pkg_install go golang go go; fi
|
if missing go; then pkg_install go golang go go; fi
|
||||||
ensure_golangci_lint
|
|
||||||
|
# Not installed here: docker is platform-specific and out of scope for a
|
||||||
|
# package-manager bootstrap, but script/lint needs it.
|
||||||
|
if missing docker; then
|
||||||
|
echo "bootstrap: docker not found; script/lint requires it" >&2
|
||||||
|
fi
|
||||||
|
|
||||||
go mod download
|
go mod download
|
||||||
|
|
||||||
|
|||||||
47
script/lint
47
script/lint
@@ -1,12 +1,55 @@
|
|||||||
#!/bin/sh
|
#!/bin/sh
|
||||||
# script/lint: run the linter.
|
# script/lint: run the linter. golangci-lint is never installed locally: it
|
||||||
|
# runs via docker only, one way, everywhere — script/lint builds
|
||||||
|
# Dockerfile.lint, which COPYs the repo into the pinned golangci-lint image
|
||||||
|
# and lints as a build step. This works even when the docker daemon is remote
|
||||||
|
# and bind mounts are impossible, and it removes the host linter's shared
|
||||||
|
# cache, which has attributed other checkouts' findings to this one.
|
||||||
|
#
|
||||||
|
# --no-cache-filter=lint forces the lint stage to re-execute on every run; a
|
||||||
|
# cached lint stage exits 0 in under a second having linted nothing. The deps
|
||||||
|
# stage keeps its cache, so module downloads are not repeated.
|
||||||
|
# --progress=plain keeps the linter's own output visible on success, so a
|
||||||
|
# passing run shows the issue count rather than nothing.
|
||||||
|
# --output=type=cacheonly leaves no image behind to clean up.
|
||||||
|
#
|
||||||
|
# docker silently ignores --no-cache-filter for a stage name that does not
|
||||||
|
# match, so a rename or a typo would restore the cached false green with no
|
||||||
|
# warning and a fast exit 0. The flag is therefore not trusted: the build
|
||||||
|
# output is teed to a log and a run is only a pass if golangci-lint's own
|
||||||
|
# summary line ("N issues." / "N issues:") is in it. No summary, no lint,
|
||||||
|
# whatever the exit code says.
|
||||||
set -eu
|
set -eu
|
||||||
|
|
||||||
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
||||||
|
|
||||||
main() {
|
main() {
|
||||||
cd "$ROOT"
|
cd "$ROOT"
|
||||||
golangci-lint run --config .golangci.yml ./...
|
|
||||||
|
log="$(mktemp -t webhooker-lint.XXXXXXXX)"
|
||||||
|
rcfile="$(mktemp -t webhooker-lint-rc.XXXXXXXX)"
|
||||||
|
trap 'rm -f "$log" "$rcfile"' EXIT INT TERM
|
||||||
|
|
||||||
|
# The pipeline's status is tee's, and POSIX sh has no pipefail, so the
|
||||||
|
# build's status travels via a file. Output still streams live.
|
||||||
|
{
|
||||||
|
docker build \
|
||||||
|
-f Dockerfile.lint \
|
||||||
|
--no-cache-filter=lint \
|
||||||
|
--progress=plain \
|
||||||
|
--output=type=cacheonly \
|
||||||
|
. 2>&1 && echo 0 >"$rcfile" || echo $? >"$rcfile"
|
||||||
|
} | tee "$log" >&2
|
||||||
|
|
||||||
|
rc="$(cat "$rcfile")"
|
||||||
|
[ "$rc" -eq 0 ] || exit "$rc"
|
||||||
|
|
||||||
|
if ! grep -qE '[0-9]+ issues[.:]' "$log"; then
|
||||||
|
echo "script/lint: golangci-lint printed no summary line; the linter" >&2
|
||||||
|
echo " did not run. Check that the stage named in --no-cache-filter" >&2
|
||||||
|
echo " still matches a stage in Dockerfile.lint." >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
}
|
}
|
||||||
|
|
||||||
main "$@"
|
main "$@"
|
||||||
|
|||||||
Reference in New Issue
Block a user