9 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
clawbotanduser	6b4a1d7607	refactor: extract magic byte detection into internal/magic package (#42) ... ## Summary Extract magic byte detection and MIME type handling from `internal/imgcache/` into a new focused `internal/magic/` package. Part of [issue #39](#39) ## Changes ### New package: `internal/magic/` Moved the following from `internal/imgcache/magic.go`: - `MIMEType` type and constants (`MIMETypeJPEG`, `MIMETypePNG`, etc.) - `DetectFormat()` — detects image format from magic bytes - `ValidateMagicBytes()` — validates content matches declared MIME type - `PeekAndValidate()` — reads minimum bytes, validates, returns combined reader - `IsSupportedMIMEType()` — checks if a MIME type is supported - `MIMEToImageFormat()` — converts MIME type to ImageFormat - `ImageFormatToMIME()` — converts ImageFormat to MIME string - All error sentinels (`ErrUnknownFormat`, `ErrMagicByteMismatch`, `ErrNotEnoughData`) - All helper functions (`detectSVG`, `skipBOM`, `normalizeMIMEType`) The magic package defines its own `ImageFormat` type and constants to avoid circular imports (`imgcache` → `magic` for validation; `magic` cannot import `imgcache`). ### Updated imports - `internal/imgcache/service.go`: uses `magic.ValidateMagicBytes()` - `internal/imgcache/service_test.go`: uses `magic.DetectFormat()` and `magic.MIMEToImageFormat()` ### Naming - Clean package-qualified names: `magic.DetectFormat()`, `magic.ValidateMagicBytes()`, etc. - No stuttering names ### Tests - Full test suite moved to `internal/magic/magic_test.go` (all 15 test functions preserved) - All existing tests pass unchanged - `docker build .` passes (includes `make check`: fmt, lint, tests) Co-authored-by: user <user@Mac.lan guest wan> Reviewed-on: #42 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>	2026-04-07 00:41:48 +02:00
3 people	e34743f070	refactor: extract whitelist package from internal/imgcache (#41) ... Extract `HostWhitelist`, `NewHostWhitelist`, `IsWhitelisted`, `IsEmpty`, and `Count` from `internal/imgcache/` into the new `internal/whitelist/` package. The whitelist package is completely self-contained, depending only on `net/url` and `strings` from the standard library. No circular imports introduced. **Changes:** - Moved `whitelist.go` → `internal/whitelist/whitelist.go` (added package comment) - Moved `whitelist_test.go` → `internal/whitelist/whitelist_test.go` (adapted to external test style) - Updated `internal/imgcache/service.go` to import from `sneak.berlin/go/pixa/internal/whitelist` `docker build .` passes (lint, tests, build). Part of [issue #39](#39) Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de> Co-authored-by: user <user@Mac.lan guest wan> Reviewed-on: #41 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>	2026-03-25 20:44:56 +01:00
6 people	7010d55d72	Move schema_migrations table creation into 000.sql (#36) ... ## Summary Moves the `schema_migrations` table definition from inline Go code into `internal/database/schema/000.sql`, so the migration tracking table schema lives alongside all other schema files. closes #29 ## Changes ### New file: `internal/database/schema/000.sql` - Contains the `CREATE TABLE IF NOT EXISTS schema_migrations` DDL - This is applied as a bootstrap step before the normal migration loop ### Refactored: `internal/database/database.go` - Removed the inline `CREATE TABLE IF NOT EXISTS schema_migrations` SQL from both `runMigrations` and `ApplyMigrations` - Added `bootstrapMigrationsTable()` which: - Checks `sqlite_master` to see if the table already exists - If missing: reads and executes `000.sql` to create it, then records version `000` - If present (backwards compat with existing DBs created by old inline code): back-fills version `000` so the normal loop skips the bootstrap file - Deduplicated: both `Database.runMigrations()` and the exported `ApplyMigrations()` now delegate to a single `applyMigrations()` helper - Added `logInfo`/`logDebug` helpers to handle the optional logger (nil when called from `ApplyMigrations` in tests) ### New file: `internal/database/database_test.go` - `TestApplyMigrations_CreatesSchemaAndTables` — verifies all migrations apply and all expected tables exist - `TestApplyMigrations_Idempotent` — verifies running migrations twice produces no errors or duplicates - `TestBootstrapMigrationsTable_FreshDatabase` — verifies bootstrap creates the table and records version 000 - `TestBootstrapMigrationsTable_ExistingTableBackwardsCompat` — verifies existing DBs (from old inline-SQL code) get version 000 back-filled without data loss ## Conflict note [PR #33](#33) (for [issue #28](#28)) is also modifying migration code. This PR is based on current `main` and the conflict will be resolved at merge time. Co-authored-by: user <user@Mac.lan guest wan> Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de> Co-authored-by: clawbot <clawbot@sneak.berlin> Co-authored-by: clawbot <clawbot@eeqj.de> Co-authored-by: Jeffrey Paul <sneak@noreply.example.org> Reviewed-on: #36 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>	2026-03-25 02:20:52 +01:00
clawbotanduser	a50364bfca	Enforce and document exact-match-only for signature verification (#40) ... Closes #27 Signatures are per-URL only — this PR adds explicit tests and documentation enforcing that HMAC-SHA256 signatures verify against exact URLs only. No suffix matching, wildcard matching, or partial matching is supported. ## What this does NOT touch **The host whitelist code (`whitelist.go`) is not modified.** This PR is exclusively about signature verification, per sneak's instructions on [issue #27](#27), [PR #32](#32), and [PR #35](#35). ## Changes ### `internal/imgcache/signature.go` - Added documentation comments on `Verify()` and `buildSignatureData()` explicitly specifying that signatures are exact-match only — no suffix, wildcard, or partial matching ### `internal/imgcache/signature_test.go` - **`TestSigner_Verify_ExactMatchOnly`**: 14 tamper cases verifying that modifying any signed component (host, path, query, dimensions, format) causes verification to fail. Host-specific cases include: - Parent domain (`example.com`) does not match subdomain signature (`cdn.example.com`) - Sibling subdomain (`images.example.com`) does not match - Deeper subdomain (`images.cdn.example.com`) does not match - Evil suffix domain (`cdn.example.com.evil.com`) does not match - Prefixed host (`evilcdn.example.com`) does not match - **`TestSigner_Sign_ExactHostInData`**: Verifies that suffix-related hosts (`cdn.example.com`, `example.com`, `images.example.com`, etc.) all produce distinct signatures ### `internal/imgcache/service_test.go` - **`TestService_ValidateRequest_SignatureExactHostMatch`**: Integration test through `ValidateRequest` verifying that a valid signature for `cdn.example.com` is rejected when presented with a different host (parent domain, sibling subdomain, deeper subdomain, evil suffix, prefixed host) ### `README.md` - Updated Signature Specification section to explicitly document exact-match-only semantics Co-authored-by: user <user@Mac.lan guest wan> Reviewed-on: #40 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>	2026-03-20 23:56:45 +01:00
3 people	e85b5ff033	Consolidate appname to internal/globals as a constant (#34) ... Closes [issue #30](#30). The appname `"pixad"` was redundantly defined in both `cmd/pixad/main.go` (as a package-level var) and `internal/globals/globals.go` (as a package-level var that got copied from main). Since the appname is always `"pixad"` and is not actually set via ldflags (only `Version` is), this PR: - Defines `appname` once as an unexported constant in `internal/globals/globals.go` - Removes the `Appname` var from `cmd/pixad/main.go` - Removes the `globals.Appname = Appname` assignment from `main.run()` - Keeps `Version` flow unchanged (still set via ldflags in main, passed to globals) The `Globals.Appname` struct field remains available to all consumers — they just get it from the constant now instead of a package var that was always `"pixad"`. All existing tests pass, `docker build .` succeeds. Co-authored-by: user <user@Mac.lan guest wan> Co-authored-by: Jeffrey Paul <sneak@noreply.example.org> Reviewed-on: #34 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>	2026-03-20 07:04:54 +01:00
3 people	55a609dd77	Bound imageprocessor.Process input read to prevent unbounded memory use (#37) ... closes #31 ## Problem `ImageProcessor.Process` used `io.ReadAll(input)` without any size limit, allowing arbitrarily large inputs to exhaust all available memory. This is a DoS vector — even though the upstream fetcher has a `MaxResponseSize` limit (50 MiB), the processor interface accepts any `io.Reader` and should defend itself independently. Additionally, the service layer's `processFromSourceOrFetch` read cached source content with `io.ReadAll` without a bound, so an unexpectedly large cached file could also cause unbounded memory consumption. ## Changes ### Processor (`processor.go`) - Added `maxInputBytes` field to `ImageProcessor` (configurable, defaults to 50 MiB via `DefaultMaxInputBytes`) - `NewImageProcessor` now accepts a `maxInputBytes` parameter (0 or negative uses the default) - `Process` now wraps the input reader with `io.LimitReader` and rejects inputs exceeding the limit with `ErrInputDataTooLarge` - Added `DefaultMaxInputBytes` and `ErrInputDataTooLarge` exported constants/errors ### Service (`service.go`) - `NewService` now wires the fetcher's `MaxResponseSize` through to the processor - Extracted `loadCachedSource` helper method to flatten nesting in `processFromSourceOrFetch` - Cached source reads are now bounded by `maxResponseSize` — oversized cached files are discarded and re-fetched ### Tests (`processor_test.go`) - `TestImageProcessor_RejectsOversizedInputData` — verifies that inputs exceeding `maxInputBytes` are rejected with `ErrInputDataTooLarge` - `TestImageProcessor_AcceptsInputWithinLimit` — verifies that inputs within the limit are processed normally - `TestImageProcessor_DefaultMaxInputBytes` — verifies that 0 and negative values use the default - All existing tests updated to use `NewImageProcessor(0)` (default limit) Co-authored-by: user <user@Mac.lan guest wan> Co-authored-by: clawbot <clawbot@eeqj.de> Reviewed-on: #37 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>	2026-03-20 07:01:15 +01:00
clawbotanduser	9c29cb57df	feat: parse version prefix from migration filenames (#33) ... 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>	2026-03-18 03:18:38 +01:00
clawbotanduser	2e934c8894	fix: QA audit fixes for 1.0/MVP readiness (#25) ... 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
clawbotandclawbot	2f15340f26	Split Dockerfile: pre-built golangci-lint stage for faster CI (#23) ... ## 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