diff --git a/Makefile b/Makefile index 67a2a5054..a3dc5132a 100644 --- a/Makefile +++ b/Makefile @@ -995,13 +995,16 @@ docker-dummy-oidc: packer-digitalocean: $(info Buildinng DigitalOcean marketplace image...) (cd ./setup/docker/cloud && packer build digitalocean.json) -lint: lint-js lint-go +lint: lint-js lint-go check-api-request-limits lint-js: $(info Linting JS code...) $(MAKE) -C frontend lint lint-go: $(info Linting Go code...) golangci-lint run --issues-exit-code 0 ./pkg/... ./internal/... ./.../internal/... +check-api-request-limits: + $(info Checking API request-body limits...) + bash ./scripts/check-api-request-limits.sh fmt-js: (cd frontend && npm run fmt) fmt-go: diff --git a/internal/api/README.md b/internal/api/README.md index 00a34c504..7a4cc5cdf 100644 --- a/internal/api/README.md +++ b/internal/api/README.md @@ -25,6 +25,7 @@ The API package exposes PhotoPrism’s HTTP endpoints via Gin handlers. Each fil - Authenticate requests using the standard middleware (`AuthRequired`) and check roles via helpers in `internal/auth/acl` (`acl.ParseRole`, `acl.ScopePermits`, `acl.ScopeAttrPermits`). - Bound request bodies before parsing JSON or multipart payloads. Use `LimitRequestBodyBytes(...)` with a route-appropriate cap before `BindJSON(...)` / `ShouldBindJSON(...)`, detect `IsRequestBodyTooLarge(err)`, and return `413 Request Entity Too Large` via `AbortRequestTooLarge(...)`. +- Keep new JSON binding sites on the shared request-limit path by running `make check-api-request-limits` (also included in `make lint`) after adding or refactoring API handlers in the root repo or private overlays. - Never log secrets or tokens. Prefer structured logging through `event.Log` and redact sensitive values before logging. - Enforce rate limiting with the shared limiters (`limiter.Auth`, `limiter.Login`) and respond with `limiter.AbortJSON` to maintain consistent 429 JSON payloads. - Derive client IPs through `api.ClientIP` and extract bearer tokens with `header.BearerToken` or the helper setters. Use constant-time comparison for tokens and secrets. diff --git a/scripts/check-api-request-limits.sh b/scripts/check-api-request-limits.sh new file mode 100755 index 000000000..d37793bca --- /dev/null +++ b/scripts/check-api-request-limits.sh @@ -0,0 +1,62 @@ +#!/usr/bin/env bash + +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +WINDOW_LINES=15 +API_DIRS=( + "internal/api" + "plus/internal/api" + "pro/internal/api" + "portal/internal/api" +) + +violations=() + +check_file() { + local file="$1" + local rel="${file#"$ROOT_DIR"/}" + local output + + output="$( + awk -v window="$WINDOW_LINES" ' + /^[[:space:]]*\/\// { next } + /^[[:space:]]*func[[:space:]]/ { last_limit = 0 } + /LimitRequestBodyBytes[[:space:]]*\(/ { last_limit = NR } + /c\.(BindJSON|ShouldBindJSON)\(/ { + if (last_limit == 0 || NR - last_limit > window) { + printf "%d:%s\n", NR, $0 + } + } + ' "$file" + )" + + if [ -z "$output" ]; then + return + fi + + while IFS= read -r line; do + violations+=("${rel}:${line}") + done <<< "$output" +} + +for dir in "${API_DIRS[@]}"; do + if [ ! -d "$ROOT_DIR/$dir" ]; then + continue + fi + + while IFS= read -r -d '' file; do + check_file "$file" + done < <(find "$ROOT_DIR/$dir" -type f -name '*.go' ! -name '*_test.go' -print0) +done + +if [ "${#violations[@]}" -gt 0 ]; then + echo "ERROR: API JSON binding without nearby request-body limit detected:" + printf ' %s\n' "${violations[@]}" + echo + echo "Add LimitRequestBodyBytes(...) before BindJSON(...) / ShouldBindJSON(...)," + echo "then handle IsRequestBodyTooLarge(err) and AbortRequestTooLarge(...)." + exit 1 +fi + +echo "OK: All reviewed API JSON binding sites have nearby request-body limits."