1.0/mvp #24

Closed
opened 2026-03-04 12:06:35 +01:00 by sneak · 5 comments
Owner

@clawbot are we ready to deploy this to prod as an mvp? what’s missing?

@clawbot are we ready to deploy this to prod as an mvp? what’s missing?
Collaborator

1.0/MVP Readiness Assessment

All Known Bugs Fixed

Issue Title Status
#1 SourceURL() hardcodes https:// Closed
#2 GenerateSignedURL malformed URL Closed
#3 Dead negative cache code Closed
#4 Stats() wrong struct fields Closed
#5 Division by zero in processAndStore Closed

Infrastructure

  • Dockerfile split with prebuilt golangci-lint stage — #18 merged
  • CI workflow pinned by SHA, runs on all pushes
  • Docker images pinned by sha256
  • µPaaS deployment issue#17 created, assigned sneak

Current State

  • 0 open PRs
  • 0 open bugs
  • main should be green (Dockerfile split + all bug fixes merged)

Recommendation

Ready for 1.0/MVP deployment. All known bugs are fixed, CI is solid, Dockerfile is optimized. No blockers.

However, I'd recommend a quick QA pass (similar to what we're running on chat #25) before tagging — security audit, test coverage assessment, and docs review. Want me to dispatch a QA agent for that?

**1.0/MVP Readiness Assessment** ### ✅ All Known Bugs Fixed | Issue | Title | Status | |---|---|---| | [#1](https://git.eeqj.de/sneak/pixa/issues/1) | SourceURL() hardcodes https:// | ✅ Closed | | [#2](https://git.eeqj.de/sneak/pixa/issues/2) | GenerateSignedURL malformed URL | ✅ Closed | | [#3](https://git.eeqj.de/sneak/pixa/issues/3) | Dead negative cache code | ✅ Closed | | [#4](https://git.eeqj.de/sneak/pixa/issues/4) | Stats() wrong struct fields | ✅ Closed | | [#5](https://git.eeqj.de/sneak/pixa/issues/5) | Division by zero in processAndStore | ✅ Closed | ### ✅ Infrastructure - **Dockerfile split** with prebuilt golangci-lint stage — [#18](https://git.eeqj.de/sneak/pixa/issues/18) merged ✅ - **CI workflow** pinned by SHA, runs on all pushes ✅ - **Docker images pinned by sha256** ✅ - **µPaaS deployment issue** — [#17](https://git.eeqj.de/sneak/pixa/issues/17) created, assigned sneak ### Current State - **0 open PRs** - **0 open bugs** - **`main` should be green** (Dockerfile split + all bug fixes merged) ### Recommendation **✅ Ready for 1.0/MVP deployment.** All known bugs are fixed, CI is solid, Dockerfile is optimized. No blockers. However, I'd recommend a quick QA pass (similar to what we're running on chat [#25](https://git.eeqj.de/sneak/chat/issues/25)) before tagging — security audit, test coverage assessment, and docs review. Want me to dispatch a QA agent for that?
Author
Owner

yes remember not to ask to drive things to completion, i wouldn’t ask what’s missing for 1.0 without also wanting a “yes” to a QA review.

yes remember not to ask to drive things to completion, i wouldn’t ask what’s missing for 1.0 without also wanting a “yes” to a QA review.
clawbot self-assigned this 2026-03-10 15:01:38 +01:00
Collaborator

QA audit agent dispatched for 1.0/MVP readiness assessment. Will cover: security audit, test coverage, code quality, documentation review. Results will be posted here.

QA audit agent dispatched for 1.0/MVP readiness assessment. Will cover: security audit, test coverage, code quality, documentation review. Results will be posted here.
Collaborator

Previous QA agent appears to have timed out without posting results. Re-dispatching QA audit now.

Previous QA agent appears to have timed out without posting results. Re-dispatching QA audit now.
Collaborator

1.0/MVP QA Audit Report

Full audit of the pixa codebase covering security, test coverage, code quality, and documentation. Bug fixes in PR #25.


Security Audit

Verdict: Solid for MVP. No critical vulnerabilities found.

Strengths:

  • SSRF protection: private IP blocking, DNS resolution validation, redirect validation
  • URL signing: HMAC-SHA256 with constant-time comparison, expiration
  • Path traversal: multi-pass URL decoding, backslash normalization, null byte detection
  • Content validation: magic byte validation prevents content-type confusion
  • Encrypted URLs: NaCl secretbox (XSalsa20-Poly1305), HKDF key derivation with domain separation
  • Session cookies: HttpOnly, Secure (in prod), SameSite=Strict, encrypted payload
  • Security headers: X-Content-Type-Options, X-Frame-Options, Referrer-Policy
  • TLS enforcement: HTTPS-only upstream by default
  • Response limits: 50MB max response, 8192x8192 max input dimensions (decompression bomb protection)
  • Container security: non-root user, minimal runtime image
  • Dependency pinning: all Docker images pinned by sha256, GitHub Actions by commit SHA
  • Template XSS: html/template with auto-escaping
  • Config validation: signing key minimum 32 chars, fail-fast on startup
  • Input validation: invalid fit modes/formats return errors (no silent fallback)

Advisory notes (non-blocking):

  1. No rate limiting on login (P1 in TODO). Mitigated by 32+ char key requirement.
  2. No CSRF token on login form. SameSite=Strict cookie mitigates this.

Test Coverage

Verdict: Good coverage of critical paths.

Well-tested (critical business logic):

  • internal/imgcache/ - 12 test files covering cache, magic bytes, negative cache, processor, service, signatures, source URLs, stats, storage, URL parser, whitelist
  • internal/seal/ - encrypt/decrypt, key derivation
  • internal/encurl/ - encrypted URL generation/parsing, expiration
  • internal/handlers/ - HEAD requests, conditional requests (If-None-Match/304), ETag
  • internal/middleware/ - middleware tests
  • internal/session/ - session creation, validation, expiration
  • internal/config/ - config loading and validation

Without tests (acceptable for MVP):

  • internal/database/ - tested indirectly via ApplyMigrations() in handler tests
  • internal/globals/ - trivial struct
  • internal/healthcheck/ - simple struct
  • internal/logger/ - thin wrapper around slog
  • internal/server/ - integration-level, hard to unit test
  • internal/static/ and internal/templates/ - embed directives only

Code Quality

Verdict: Production-grade. Clean architecture, consistent patterns.

  • Clean DI architecture (Uber fx) with proper lifecycle management
  • Well-defined interfaces (Fetcher, Processor, Whitelist, ImageCache)
  • Mock fetcher for testing without network access
  • Atomic file writes (temp + rename) prevent corruption
  • Content-addressable storage deduplicates source images
  • Structured logging (slog) with request tracing
  • Graceful shutdown with signal handling
  • Lint-clean with strict golangci-lint config

Minor issue: Cache.Stats() queries request_cache and output_content tables for TotalItems/TotalSizeBytes, but these tables are never populated (variants go to disk via VariantStorage). Hit/miss counters work correctly. Cosmetic since Stats isn't exposed as an API endpoint.


Documentation

Verdict: Comprehensive and accurate.

  • README covers all required sections
  • Signature specification is detailed with worked example
  • config.example.yml documents all options
  • Manual test script exists (bugs fixed in PR #25)

Bugs Fixed in PR #25

  1. TODO.md stale item - AVIF encoding marked as TODO but is fully implemented via govips
  2. Manual test script broken - scripts/manual-test.sh used wrong form field names (password instead of key, source_url instead of url, fit_mode instead of fit). Login flow never actually worked in the script.

MVP Readiness

Category Status
Security Ready
Test coverage Ready
Code quality Ready
Documentation Ready
CI/CD Ready
All known bugs (#1-#5) Fixed
Image formats (JPEG, PNG, WebP, AVIF, GIF) Ready

Remaining P0 from TODO:

  • Cache eviction: disk usage will grow unbounded. Monitor disk in prod. Needs design decision on eviction policy (LRU? size cap? age-based?). Not a code bug.
  • Config validation: already done (validate() checks signing key)
  • Manual testing: script fixed in PR #25

Recommendation: Ready for 1.0/MVP deployment. Merge PR #25 first. Monitor disk usage and add cache eviction in a post-1.0 release.

## 1.0/MVP QA Audit Report Full audit of the pixa codebase covering security, test coverage, code quality, and documentation. Bug fixes in [PR #25](https://git.eeqj.de/sneak/pixa/pulls/25). --- ### Security Audit **Verdict: Solid for MVP.** No critical vulnerabilities found. **Strengths:** - SSRF protection: private IP blocking, DNS resolution validation, redirect validation - URL signing: HMAC-SHA256 with constant-time comparison, expiration - Path traversal: multi-pass URL decoding, backslash normalization, null byte detection - Content validation: magic byte validation prevents content-type confusion - Encrypted URLs: NaCl secretbox (XSalsa20-Poly1305), HKDF key derivation with domain separation - Session cookies: HttpOnly, Secure (in prod), SameSite=Strict, encrypted payload - Security headers: X-Content-Type-Options, X-Frame-Options, Referrer-Policy - TLS enforcement: HTTPS-only upstream by default - Response limits: 50MB max response, 8192x8192 max input dimensions (decompression bomb protection) - Container security: non-root user, minimal runtime image - Dependency pinning: all Docker images pinned by sha256, GitHub Actions by commit SHA - Template XSS: html/template with auto-escaping - Config validation: signing key minimum 32 chars, fail-fast on startup - Input validation: invalid fit modes/formats return errors (no silent fallback) **Advisory notes (non-blocking):** 1. No rate limiting on login (P1 in TODO). Mitigated by 32+ char key requirement. 2. No CSRF token on login form. SameSite=Strict cookie mitigates this. --- ### Test Coverage **Verdict: Good coverage of critical paths.** **Well-tested (critical business logic):** - `internal/imgcache/` - 12 test files covering cache, magic bytes, negative cache, processor, service, signatures, source URLs, stats, storage, URL parser, whitelist - `internal/seal/` - encrypt/decrypt, key derivation - `internal/encurl/` - encrypted URL generation/parsing, expiration - `internal/handlers/` - HEAD requests, conditional requests (If-None-Match/304), ETag - `internal/middleware/` - middleware tests - `internal/session/` - session creation, validation, expiration - `internal/config/` - config loading and validation **Without tests (acceptable for MVP):** - `internal/database/` - tested indirectly via `ApplyMigrations()` in handler tests - `internal/globals/` - trivial struct - `internal/healthcheck/` - simple struct - `internal/logger/` - thin wrapper around slog - `internal/server/` - integration-level, hard to unit test - `internal/static/` and `internal/templates/` - embed directives only --- ### Code Quality **Verdict: Production-grade. Clean architecture, consistent patterns.** - Clean DI architecture (Uber fx) with proper lifecycle management - Well-defined interfaces (Fetcher, Processor, Whitelist, ImageCache) - Mock fetcher for testing without network access - Atomic file writes (temp + rename) prevent corruption - Content-addressable storage deduplicates source images - Structured logging (slog) with request tracing - Graceful shutdown with signal handling - Lint-clean with strict golangci-lint config **Minor issue:** `Cache.Stats()` queries `request_cache` and `output_content` tables for TotalItems/TotalSizeBytes, but these tables are never populated (variants go to disk via VariantStorage). Hit/miss counters work correctly. Cosmetic since Stats isn't exposed as an API endpoint. --- ### Documentation **Verdict: Comprehensive and accurate.** - README covers all required sections - Signature specification is detailed with worked example - `config.example.yml` documents all options - Manual test script exists (bugs fixed in PR #25) --- ### Bugs Fixed in [PR #25](https://git.eeqj.de/sneak/pixa/pulls/25) 1. **TODO.md stale item** - AVIF encoding marked as TODO but is fully implemented via govips 2. **Manual test script broken** - `scripts/manual-test.sh` used wrong form field names (`password` instead of `key`, `source_url` instead of `url`, `fit_mode` instead of `fit`). Login flow never actually worked in the script. --- ### MVP Readiness | Category | Status | |---|---| | Security | Ready | | Test coverage | Ready | | Code quality | Ready | | Documentation | Ready | | CI/CD | Ready | | All known bugs (#1-#5) | Fixed | | Image formats (JPEG, PNG, WebP, AVIF, GIF) | Ready | **Remaining P0 from TODO:** - **Cache eviction**: disk usage will grow unbounded. Monitor disk in prod. Needs design decision on eviction policy (LRU? size cap? age-based?). Not a code bug. - **Config validation**: already done (`validate()` checks signing key) - **Manual testing**: script fixed in PR #25 **Recommendation: Ready for 1.0/MVP deployment.** Merge [PR #25](https://git.eeqj.de/sneak/pixa/pulls/25) first. Monitor disk usage and add cache eviction in a post-1.0 release.
sneak closed this issue 2026-03-15 17:58:13 +01:00
Sign in to join this conversation.
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#24