3 Commits

Author SHA1 Message Date
user
871972f726 bound imageprocessor.Process input read to prevent unbounded memory use
All checks were successful
check / check (push) Successful in 1m9s
ImageProcessor.Process used io.ReadAll without a size limit, allowing
arbitrarily large inputs to exhaust memory. Add a configurable
maxInputBytes limit (default 50 MiB, matching the fetcher limit) and
reject inputs that exceed it with ErrInputDataTooLarge.

Also bound the cached source content read in the service layer to
prevent unexpectedly large cached files from consuming unbounded memory.

Extracted loadCachedSource helper to reduce nesting complexity.
2026-03-17 01:59:15 -07:00
2e934c8894 fix: QA audit fixes for 1.0/MVP readiness (#25)
All checks were successful
check / check (push) Successful in 5s
closes #24

## QA Audit Fixes

This PR addresses issues found during the 1.0/MVP QA audit.

### Changes

1. **TODO.md: Mark AVIF encoding as done** — AVIF encoding is fully implemented via govips in `processor.go` but was still listed as a TODO item.

2. **scripts/manual-test.sh: Fix form field names** — The manual test script was using wrong field names:
   - Login form: was sending `password=...`, should be `key=...` (matching the HTML form's `name="key"`)
   - Generator form: was sending `source_url`, `fit_mode` — should be `url`, `fit` (matching the handler's `r.FormValue()` calls)
   - This means **the manual test script never actually worked** — login always failed silently because the `key` field was empty.

### Full QA Audit Results

The comprehensive QA audit report has been posted as a comment on [issue #24](#24).

Co-authored-by: user <user@Mac.lan guest wan>
Reviewed-on: #25
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
2026-03-15 17:58:13 +01:00
2f15340f26 Split Dockerfile: pre-built golangci-lint stage for faster CI (#23)
All checks were successful
check / check (push) Successful in 5s
## Summary

Splits the Dockerfile into a dedicated lint stage using the pre-built `golangci/golangci-lint:v2.10.1-alpine` Docker image, replacing the manual binary download with curl/sha256 verification.

## Changes

- **Lint stage** (`AS lint`): Uses `golangci/golangci-lint:v2.10.1-alpine` pinned by sha256. Runs `make fmt-check` + `make lint`. Includes CGO deps (`build-base`, `vips-dev`, `libheif-dev`, `pkgconfig`) needed for type-checking govips imports.
- **Build stage** (`AS builder`): Depends on lint stage via `COPY --from=lint /src/go.sum /dev/null`. Runs `make test` + builds the binary. Removes `curl` (no longer needed) and the manual golangci-lint download block.
- **Runtime stage**: Unchanged.

## Benefits

- Eliminates slow multi-arch binary download + sha256 verification step
- Lint and build stages can potentially run in parallel with BuildKit
- Better Docker layer caching — lint deps cached separately from build deps
- All images remain pinned by sha256 with version+date comments

## Verification

- `docker build .` passes: fmt-check , lint (0 issues) , all tests pass , binary builds 

Closes [#18](#18)

<!-- session: agent:sdlc-manager:subagent:7aac9c54-81c8-4494-94ab-0843f97a1e62 -->

Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de>
Reviewed-on: #23
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
2026-03-02 21:09:51 +01:00
6 changed files with 187 additions and 73 deletions

View File

@@ -1,32 +1,31 @@
# Lint stage — fast feedback on formatting and lint issues
# golangci/golangci-lint:v2.10.1, 2026-03-01
FROM golangci/golangci-lint@sha256:ea84d14c2fef724411be7dc45e09e6ef721d748315252b02df19a7e3113ee763 AS lint
# Lint stage
# golangci/golangci-lint:v2.10.1-alpine, 2026-02-17
FROM golangci/golangci-lint:v2.10.1-alpine@sha256:33bc6b6156d4c7da87175f187090019769903d04dd408833b83083ed214b0ddf AS lint
# Install CGO dependencies needed for static analysis of vips/libheif code
RUN apt-get update && apt-get install -y --no-install-recommends \
libvips-dev \
libheif-dev \
pkg-config \
&& rm -rf /var/lib/apt/lists/*
RUN apk add --no-cache make build-base vips-dev libheif-dev pkgconfig
WORKDIR /src
# Copy go mod files first for better layer caching
COPY go.mod go.sum ./
RUN go mod download
# Copy source code
COPY . .
# Run formatting check and linter
RUN make fmt-check
RUN make lint
# Build stage — tests and compilation
# Build stage
# golang:1.25.4-alpine, 2026-02-25
FROM golang:1.25.4-alpine@sha256:d3f0cf7723f3429e3f9ed846243970b20a2de7bae6a5b66fc5914e228d831bbb AS builder
ARG VERSION=dev
# Force BuildKit to run the lint stage by creating a stage dependency
# Depend on lint stage passing
COPY --from=lint /src/go.sum /dev/null
ARG VERSION=dev
# Install build dependencies for CGO image libraries
RUN apk add --no-cache \
build-base \

View File

@@ -6,7 +6,7 @@ Remaining tasks sorted by priority for a working 1.0 release.
### Image Processing
- [x] Add WebP encoding support (currently returns error)
- [ ] Add AVIF encoding support (currently returns error)
- [x] Add AVIF encoding support (implemented via govips)
### Manual Testing (verify auth/encrypted URLs work)
- [ ] Manual test: visit `/`, see login form

View File

@@ -26,20 +26,36 @@ func initVips() {
// Images larger than this are rejected to prevent DoS via decompression bombs.
const MaxInputDimension = 8192
// DefaultMaxInputBytes is the default maximum input size in bytes (50 MiB).
// This matches the default upstream fetcher limit.
const DefaultMaxInputBytes = 50 << 20
// ErrInputTooLarge is returned when input image dimensions exceed MaxInputDimension.
var ErrInputTooLarge = errors.New("input image dimensions exceed maximum")
// ErrInputDataTooLarge is returned when the raw input data exceeds the configured byte limit.
var ErrInputDataTooLarge = errors.New("input data exceeds maximum allowed size")
// ErrUnsupportedOutputFormat is returned when the requested output format is not supported.
var ErrUnsupportedOutputFormat = errors.New("unsupported output format")
// ImageProcessor implements the Processor interface using libvips via govips.
type ImageProcessor struct{}
type ImageProcessor struct {
maxInputBytes int64
}
// NewImageProcessor creates a new image processor.
func NewImageProcessor() *ImageProcessor {
// NewImageProcessor creates a new image processor with the given maximum input
// size in bytes. If maxInputBytes is <= 0, DefaultMaxInputBytes is used.
func NewImageProcessor(maxInputBytes int64) *ImageProcessor {
initVips()
return &ImageProcessor{}
if maxInputBytes <= 0 {
maxInputBytes = DefaultMaxInputBytes
}
return &ImageProcessor{
maxInputBytes: maxInputBytes,
}
}
// Process transforms an image according to the request.
@@ -48,12 +64,20 @@ func (p *ImageProcessor) Process(
input io.Reader,
req *ImageRequest,
) (*ProcessResult, error) {
// Read input
data, err := io.ReadAll(input)
// Read input with a size limit to prevent unbounded memory consumption.
// We read at most maxInputBytes+1 so we can detect if the input exceeds
// the limit without consuming additional memory.
limited := io.LimitReader(input, p.maxInputBytes+1)
data, err := io.ReadAll(limited)
if err != nil {
return nil, fmt.Errorf("failed to read input: %w", err)
}
if int64(len(data)) > p.maxInputBytes {
return nil, ErrInputDataTooLarge
}
// Decode image
img, err := vips.NewImageFromBuffer(data)
if err != nil {

View File

@@ -71,7 +71,7 @@ func createTestPNG(t *testing.T, width, height int) []byte {
}
func TestImageProcessor_ResizeJPEG(t *testing.T) {
proc := NewImageProcessor()
proc := NewImageProcessor(0)
ctx := context.Background()
input := createTestJPEG(t, 800, 600)
@@ -118,7 +118,7 @@ func TestImageProcessor_ResizeJPEG(t *testing.T) {
}
func TestImageProcessor_ConvertToPNG(t *testing.T) {
proc := NewImageProcessor()
proc := NewImageProcessor(0)
ctx := context.Background()
input := createTestJPEG(t, 200, 150)
@@ -151,7 +151,7 @@ func TestImageProcessor_ConvertToPNG(t *testing.T) {
}
func TestImageProcessor_OriginalSize(t *testing.T) {
proc := NewImageProcessor()
proc := NewImageProcessor(0)
ctx := context.Background()
input := createTestJPEG(t, 640, 480)
@@ -179,7 +179,7 @@ func TestImageProcessor_OriginalSize(t *testing.T) {
}
func TestImageProcessor_FitContain(t *testing.T) {
proc := NewImageProcessor()
proc := NewImageProcessor(0)
ctx := context.Background()
// 800x400 image (2:1 aspect) into 400x400 box with contain
@@ -206,7 +206,7 @@ func TestImageProcessor_FitContain(t *testing.T) {
}
func TestImageProcessor_ProportionalScale_WidthOnly(t *testing.T) {
proc := NewImageProcessor()
proc := NewImageProcessor(0)
ctx := context.Background()
// 800x600 image, request width=400 height=0
@@ -236,7 +236,7 @@ func TestImageProcessor_ProportionalScale_WidthOnly(t *testing.T) {
}
func TestImageProcessor_ProportionalScale_HeightOnly(t *testing.T) {
proc := NewImageProcessor()
proc := NewImageProcessor(0)
ctx := context.Background()
// 800x600 image, request width=0 height=300
@@ -266,7 +266,7 @@ func TestImageProcessor_ProportionalScale_HeightOnly(t *testing.T) {
}
func TestImageProcessor_ProcessPNG(t *testing.T) {
proc := NewImageProcessor()
proc := NewImageProcessor(0)
ctx := context.Background()
input := createTestPNG(t, 400, 300)
@@ -298,7 +298,7 @@ func TestImageProcessor_ImplementsInterface(t *testing.T) {
}
func TestImageProcessor_SupportedFormats(t *testing.T) {
proc := NewImageProcessor()
proc := NewImageProcessor(0)
inputFormats := proc.SupportedInputFormats()
if len(inputFormats) == 0 {
@@ -312,7 +312,7 @@ func TestImageProcessor_SupportedFormats(t *testing.T) {
}
func TestImageProcessor_RejectsOversizedInput(t *testing.T) {
proc := NewImageProcessor()
proc := NewImageProcessor(0)
ctx := context.Background()
// Create an image that exceeds MaxInputDimension (e.g., 10000x100)
@@ -337,7 +337,7 @@ func TestImageProcessor_RejectsOversizedInput(t *testing.T) {
}
func TestImageProcessor_RejectsOversizedInputHeight(t *testing.T) {
proc := NewImageProcessor()
proc := NewImageProcessor(0)
ctx := context.Background()
// Create an image with oversized height
@@ -361,7 +361,7 @@ func TestImageProcessor_RejectsOversizedInputHeight(t *testing.T) {
}
func TestImageProcessor_AcceptsMaxDimensionInput(t *testing.T) {
proc := NewImageProcessor()
proc := NewImageProcessor(0)
ctx := context.Background()
// Create an image at exactly MaxInputDimension - should be accepted
@@ -383,7 +383,7 @@ func TestImageProcessor_AcceptsMaxDimensionInput(t *testing.T) {
}
func TestImageProcessor_EncodeWebP(t *testing.T) {
proc := NewImageProcessor()
proc := NewImageProcessor(0)
ctx := context.Background()
input := createTestJPEG(t, 200, 150)
@@ -426,7 +426,7 @@ func TestImageProcessor_EncodeWebP(t *testing.T) {
}
func TestImageProcessor_DecodeAVIF(t *testing.T) {
proc := NewImageProcessor()
proc := NewImageProcessor(0)
ctx := context.Background()
// Load test AVIF file
@@ -465,8 +465,73 @@ func TestImageProcessor_DecodeAVIF(t *testing.T) {
}
}
func TestImageProcessor_RejectsOversizedInputData(t *testing.T) {
// Create a processor with a very small byte limit
const limit = 1024
proc := NewImageProcessor(limit)
ctx := context.Background()
// Create a valid JPEG that exceeds the byte limit
input := createTestJPEG(t, 800, 600) // will be well over 1 KiB
if int64(len(input)) <= limit {
t.Fatalf("test JPEG must exceed %d bytes, got %d", limit, len(input))
}
req := &ImageRequest{
Size: Size{Width: 100, Height: 75},
Format: FormatJPEG,
Quality: 85,
FitMode: FitCover,
}
_, err := proc.Process(ctx, bytes.NewReader(input), req)
if err == nil {
t.Fatal("Process() should reject input exceeding maxInputBytes")
}
if err != ErrInputDataTooLarge {
t.Errorf("Process() error = %v, want ErrInputDataTooLarge", err)
}
}
func TestImageProcessor_AcceptsInputWithinLimit(t *testing.T) {
// Create a small image and set limit well above its size
input := createTestJPEG(t, 10, 10)
limit := int64(len(input)) * 10 // 10× headroom
proc := NewImageProcessor(limit)
ctx := context.Background()
req := &ImageRequest{
Size: Size{Width: 10, Height: 10},
Format: FormatJPEG,
Quality: 85,
FitMode: FitCover,
}
result, err := proc.Process(ctx, bytes.NewReader(input), req)
if err != nil {
t.Fatalf("Process() error = %v, want nil", err)
}
defer result.Content.Close()
}
func TestImageProcessor_DefaultMaxInputBytes(t *testing.T) {
// Passing 0 should use the default
proc := NewImageProcessor(0)
if proc.maxInputBytes != DefaultMaxInputBytes {
t.Errorf("maxInputBytes = %d, want %d", proc.maxInputBytes, DefaultMaxInputBytes)
}
// Passing negative should also use the default
proc = NewImageProcessor(-1)
if proc.maxInputBytes != DefaultMaxInputBytes {
t.Errorf("maxInputBytes = %d, want %d", proc.maxInputBytes, DefaultMaxInputBytes)
}
}
func TestImageProcessor_EncodeAVIF(t *testing.T) {
proc := NewImageProcessor()
proc := NewImageProcessor(0)
ctx := context.Background()
input := createTestJPEG(t, 200, 150)

View File

@@ -22,6 +22,7 @@ type Service struct {
whitelist *HostWhitelist
log *slog.Logger
allowHTTP bool
maxResponseSize int64
}
// ServiceConfig holds configuration for the image service.
@@ -50,15 +51,17 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
return nil, errors.New("signing key is required")
}
// Resolve fetcher config for defaults
fetcherCfg := cfg.FetcherConfig
if fetcherCfg == nil {
fetcherCfg = DefaultFetcherConfig()
}
// Use custom fetcher if provided, otherwise create HTTP fetcher
var fetcher Fetcher
if cfg.Fetcher != nil {
fetcher = cfg.Fetcher
} else {
fetcherCfg := cfg.FetcherConfig
if fetcherCfg == nil {
fetcherCfg = DefaultFetcherConfig()
}
fetcher = NewHTTPFetcher(fetcherCfg)
}
@@ -74,14 +77,17 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
allowHTTP = cfg.FetcherConfig.AllowHTTP
}
maxResponseSize := fetcherCfg.MaxResponseSize
return &Service{
cache: cfg.Cache,
fetcher: fetcher,
processor: NewImageProcessor(),
processor: NewImageProcessor(maxResponseSize),
signer: signer,
whitelist: NewHostWhitelist(cfg.Whitelist),
log: log,
allowHTTP: allowHTTP,
maxResponseSize: maxResponseSize,
}, nil
}
@@ -146,6 +152,40 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
return response, nil
}
// loadCachedSource attempts to load source content from cache, returning nil
// if the cached data is unavailable or exceeds maxResponseSize.
func (s *Service) loadCachedSource(contentHash ContentHash) []byte {
reader, err := s.cache.GetSourceContent(contentHash)
if err != nil {
s.log.Warn("failed to load cached source, fetching", "error", err)
return nil
}
// Bound the read to maxResponseSize to prevent unbounded memory use
// from unexpectedly large cached files.
limited := io.LimitReader(reader, s.maxResponseSize+1)
data, err := io.ReadAll(limited)
_ = reader.Close()
if err != nil {
s.log.Warn("failed to read cached source, fetching", "error", err)
return nil
}
if int64(len(data)) > s.maxResponseSize {
s.log.Warn("cached source exceeds max response size, discarding",
"hash", contentHash,
"max_bytes", s.maxResponseSize,
)
return nil
}
return data
}
// processFromSourceOrFetch processes an image, using cached source content if available.
func (s *Service) processFromSourceOrFetch(
ctx context.Context,
@@ -162,22 +202,8 @@ func (s *Service) processFromSourceOrFetch(
var fetchBytes int64
if contentHash != "" {
// We have cached source - load it
s.log.Debug("using cached source", "hash", contentHash)
reader, err := s.cache.GetSourceContent(contentHash)
if err != nil {
s.log.Warn("failed to load cached source, fetching", "error", err)
// Fall through to fetch
} else {
sourceData, err = io.ReadAll(reader)
_ = reader.Close()
if err != nil {
s.log.Warn("failed to read cached source, fetching", "error", err)
// Fall through to fetch
}
}
sourceData = s.loadCachedSource(contentHash)
}
// Fetch from upstream if we don't have source data or it's empty

View File

@@ -48,7 +48,7 @@ fi
# Test 3: Wrong password shows error
echo "--- Test 3: Login with wrong password ---"
WRONG_LOGIN=$(curl -sf -X POST "$BASE_URL/" -d "password=wrong-key" -c "$COOKIE_JAR")
WRONG_LOGIN=$(curl -sf -X POST "$BASE_URL/" -d "key=wrong-key" -c "$COOKIE_JAR")
if echo "$WRONG_LOGIN" | grep -qi "invalid\|error\|incorrect\|wrong"; then
pass "Wrong password shows error message"
else
@@ -57,7 +57,7 @@ fi
# Test 4: Correct password redirects to generator
echo "--- Test 4: Login with correct signing key ---"
curl -sf -X POST "$BASE_URL/" -d "password=$SIGNING_KEY" -c "$COOKIE_JAR" -b "$COOKIE_JAR" -L -o /dev/null
curl -sf -X POST "$BASE_URL/" -d "key=$SIGNING_KEY" -c "$COOKIE_JAR" -b "$COOKIE_JAR" -L -o /dev/null
GENERATOR_PAGE=$(curl -sf "$BASE_URL/" -b "$COOKIE_JAR")
if echo "$GENERATOR_PAGE" | grep -qi "generate\|url\|source\|logout"; then
pass "Correct password shows generator page"
@@ -68,12 +68,12 @@ fi
# Test 5: Generate encrypted URL
echo "--- Test 5: Generate encrypted URL ---"
GEN_RESULT=$(curl -sf -X POST "$BASE_URL/generate" -b "$COOKIE_JAR" \
-d "source_url=$TEST_IMAGE_URL" \
-d "url=$TEST_IMAGE_URL" \
-d "width=800" \
-d "height=600" \
-d "format=jpeg" \
-d "quality=85" \
-d "fit_mode=cover" \
-d "fit=cover" \
-d "ttl=3600")
if echo "$GEN_RESULT" | grep -q "/v1/e/"; then
pass "Encrypted URL generated"
@@ -121,10 +121,10 @@ fi
# Test 9: Generate short-TTL URL and verify expiration
echo "--- Test 9: Expired URL returns 410 ---"
# Login again
curl -sf -X POST "$BASE_URL/" -d "password=$SIGNING_KEY" -c "$COOKIE_JAR" -b "$COOKIE_JAR" -L -o /dev/null
curl -sf -X POST "$BASE_URL/" -d "key=$SIGNING_KEY" -c "$COOKIE_JAR" -b "$COOKIE_JAR" -L -o /dev/null
# Generate URL with 1 second TTL
GEN_RESULT=$(curl -sf -X POST "$BASE_URL/generate" -b "$COOKIE_JAR" \
-d "source_url=$TEST_IMAGE_URL" \
-d "url=$TEST_IMAGE_URL" \
-d "width=100" \
-d "height=100" \
-d "format=jpeg" \