Move ImageProcessor, Params, New(), DefaultMaxInputBytes, ErrInputDataTooLarge,
and related types from internal/imgcache/ into a new standalone package
internal/imageprocessor/.
The imageprocessor package defines its own Format, FitMode, Size, Request, and
Result types, making it fully independent with no imports from imgcache. The
imgcache service converts between its own types and imageprocessor types at the
boundary.
Changes:
- New package: internal/imageprocessor/ with imageprocessor.go and tests
- Removed: processor.go and processor_test.go from internal/imgcache/
- Removed: Processor interface and ProcessResult from imgcache.go (now unused)
- Updated: service.go uses *imageprocessor.ImageProcessor directly
- Copied: testdata/red.avif for AVIF decode test
Addresses review feedback on PR #37: image processing is a distinct concern
from the HTTP service layer and belongs in its own package.
Rename NewImageProcessor(maxInputBytes) to New(Params{}) with a Params
struct containing MaxInputBytes. Zero-value Params{} uses sensible
defaults (DefaultMaxInputBytes). All callers updated.
Addresses review feedback on PR #37.
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.
Closes#28
Migration filenames now follow the pattern `<version>_<description>.sql` (e.g. `001_initial_schema.sql`). The version stored in `schema_migrations` is the numeric prefix only, not the full filename stem.
## Changes
- **`ParseMigrationVersion()`** — new exported function that extracts the numeric prefix from migration filenames. Validates that the prefix is purely numeric and rejects malformed filenames (empty prefix, non-numeric characters, leading underscore).
- **Renamed `001.sql` → `001_initial_schema.sql`** — migration files can now have descriptive names while the tracked version remains `001`. This is safe pre-1.0.0 (no installed base).
- **Deduplicated migration logic** — `runMigrations()` and `ApplyMigrations()` now share a single `applyMigrations()` implementation, plus extracted `collectMigrations()` and `ensureMigrationsTable()` helpers.
- **Unit tests** — `TestParseMigrationVersion` covers valid patterns (version-only, with description, multi-digit, multiple underscores) and error cases (empty, leading underscore, non-numeric, mixed alphanumeric). `TestApplyMigrations` and `TestApplyMigrationsIdempotent` verify end-to-end migration application against an in-memory SQLite database.
Co-authored-by: user <user@Mac.lan guest wan>
Reviewed-on: #33
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
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>
The golangci-lint binary was hardcoded as linux-amd64, causing Docker builds
to fail on arm64 hosts. The amd64 ELF binary cannot execute on aarch64,
producing a misleading shell syntax error during make check.
Use uname -m to detect the container architecture at build time and download
the matching binary. Both amd64 and arm64 SHA-256 hashes are pinned.
Closes#15
- Update Dockerfile base image from golang:1.24-alpine to golang:1.25.4-alpine
(pinned by sha256 digest) to match go.mod requirement of go >= 1.25.4
- Fix gosec G703 (path traversal) false positives by adding filepath.Clean()
at call sites with nolint annotations for internally-constructed paths
- Fix gosec G704 (SSRF) false positive with nolint annotation; URL is already
validated by validateURL() which checks scheme, resolves DNS, and blocks
private IPs
- All make check passes clean (lint + tests)
- Add blank lines before return statements (nlreturn)
- Remove unused metaCacheMu field and sync import (unused)
- Rename unused groups parameter to _ (revive)
- Use StorageFilePerm constant instead of magic 0600 (mnd, gosec)
- Add nolint directive for vipsOnce global (gochecknoglobals)
Remove config.yaml and config.dev.yml (local development configs with
hardcoded keys that shouldn't be committed). config.example.yml remains
as the canonical example config. Added removed files to .gitignore.
SourceURL() previously hardcoded https:// regardless of the AllowHTTP
config setting. This made testing with HTTP-only test servers impossible.
Add AllowHTTP field to ImageRequest and use it to determine the URL
scheme. The Service propagates the config setting to each request.
Fixes#1
The checkNegativeCache() method existed but was never called, making
negative caching (for failed fetches) completely non-functional.
Failed URLs were being re-fetched on every request.
Add negative cache check at the start of Service.Get() to short-circuit
requests for recently-failed URLs.
Fixes#3
processAndStore() computed sizePercent as outputSize/fetchBytes*100
without checking for zero, producing Inf/NaN in logs and metrics.
Also treat empty cached source data the same as missing (re-fetch
from upstream) since zero-byte images can't be processed.
Fixes#5
Stats() was scanning 5 SQL columns (hit_count, miss_count,
upstream_fetch_count, upstream_fetch_bytes, transform_count) into
mismatched struct fields, causing HitRate to contain the integer
transform_count instead of a 0.0-1.0 ratio.
Simplify the query to only fetch hit_count and miss_count, then
compute TotalItems, TotalSizeBytes, and HitRate correctly.
Fixes#4
When a source URL has query parameters, GenerateSignedURL() was
embedding a bare '?' in the path, causing everything after it to be
parsed as the HTTP query string instead of as path segments. This
made the size/format segment unreachable by the URL parser.
Percent-encode the query string in the path segment so it remains
part of the path and can be correctly extracted by ParseImagePath.
Fixes#2
Define ContentHash, VariantKey, and PathHash types to replace
raw strings, providing compile-time type safety for storage
operations. Update storage layer to use typed parameters,
refactor cache to use variant storage keyed by VariantKey,
and implement source content reuse on cache misses.
Add trailing filename to encrypted URLs for better browser compatibility.
The filename is ignored by the server but helps browsers identify content type.
Since signing_key is now required at config load time, sessMgr, encGen,
and signer are always initialized. Remove unnecessary nil checks that
were runtime failure paths that can no longer be reached.
- handlers.go: Remove conditional init, always create sessMgr/encGen
- auth.go: Remove nil checks for sessMgr
- imageenc.go: Remove nil check for encGen
- service.go: Require signing_key in NewService, remove signer nil checks
- Update tests to provide signing_key
- Add config validation: signing_key required, minimum 32 characters
- Server now fails to start without valid signing_key (no more runtime errors)
- Add config.example.yml with default whitelist hosts
- Copy config to /etc/pixa/config.yml in Docker image
- Update entrypoint to use --config /etc/pixa/config.yml
- Add config.dev.yml for local Docker development
- Mount dev config in make devserver
- Change default StateDir from ./data to /var/lib/pixa (proper Unix convention)
- Create directory owned by pixad user in Dockerfile
- Set WORKDIR to /var/lib/pixa
- Replace gen2brain/avif, gen2brain/webp, disintegration/imaging with govips
- govips uses libvips via CGO for fast native image processing
- Add libheif-dev to Dockerfile for AVIF support
- Add docker-test Makefile target for running tests in Docker
- Update processor.go to use vips API for decode, resize, encode
- Add TestMain to initialize/shutdown vips in tests
- Remove WASM-based libraries (gen2brain) in favor of native codecs
Performance improvement: AVIF encoding now uses native libheif instead of
WASM, significantly reducing encoding time for large images.
- Build stage: golang:1.24-alpine with vips-dev for CGO image libs
- Runtime stage: alpine:3.21 with vips runtime only
- Pass VERSION build arg for ldflags embedding
- Add 'make docker' target to build image with git version
