feat(aiometadata): add optional poster reverse-proxy cache - #2
Conversation
WalkthroughThis PR adds poster-cache hostname wiring, an optional poster reverse proxy cache configuration block, a new nginx-based poster-cache service, Authelia access rules for its admin paths, and supporting purge and statistics scripts. ChangesPoster reverse proxy cache
Sequence Diagram(s)sequenceDiagram
participant Client
participant "poster-cache nginx" as Nginx
participant "dynamic upstream URL" as Upstream
participant "poster-cache-purge-handler.sh" as PurgeHandler
participant "poster-cache-stats.sh" as Stats
participant "/tmp/purge-cache" as PurgeFlag
participant "/tmp/cache-stats.json" as CacheStats
Client->>Nginx: GET poster image
alt cache miss
Nginx->>Upstream: proxy request
Upstream-->>Nginx: poster response
end
Nginx-->>Client: cached response with X-Cache-Status
Client->>Nginx: POST /purge
Nginx->>PurgeHandler: forward purge request
PurgeHandler->>PurgeFlag: create purge flag
Nginx->>CacheStats: serve /stats
loop every 30 seconds
Stats->>PurgeFlag: check for purge flag
Stats->>CacheStats: write cache statistics JSON
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/aiometadata/poster-cache-nginx.conf`:
- Around line 67-76: The redirect handling in the poster-cache nginx config is
sending clients to the upstream origin instead of keeping them on the proxy,
which bypasses caching for relative 302s. Update the proxy_redirect rule in
poster-cache-nginx.conf so redirects from upstreams like openposterdb are
rewritten to the proxy URL/path rather than $upstream_origin, and keep the
behavior localized to the redirect rewrite block near proxy_cache and
proxy_cache_key.
- Around line 51-55: The /purge location in poster-cache-nginx.conf is publicly
reachable and currently accepts state-changing requests via GET, so update the
nginx routing to keep poster fetches public but restrict purge access through an
internal-only path or a shared secret. In the location = /purge block, require a
non-GET method and add an access control check before proxying to
127.0.0.1:9888, using the existing purge endpoint configuration as the place to
enforce the protection.
- Around line 21-25: The request-to-upstream mapping in poster-cache is
currently driven directly by $request_uri, which allows arbitrary HTTP(S)
targets and creates an open proxy/SSRF path. Update the nginx configuration
around the $request_uri map and proxy_pass flow to enforce a strict allowlist of
trusted poster origins, or require signed URLs before selecting any upstream.
Make sure the upstream selection logic only resolves to approved poster CDN
domains and never forwards user-supplied full URLs verbatim.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 906a8ffd-e8cc-4d7b-9ddb-01018b63a8c2
📒 Files selected for processing (6)
.envapps/aiometadata/.envapps/aiometadata/compose.yamlapps/aiometadata/poster-cache-nginx.confapps/aiometadata/poster-cache-purge-handler.shapps/aiometadata/poster-cache-stats.sh
📜 Review details
🧰 Additional context used
🪛 dotenv-linter (4.0.0)
apps/aiometadata/.env
[warning] 224-224: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
🪛 Shellcheck (0.11.0)
apps/aiometadata/poster-cache-purge-handler.sh
[warning] 3-3: method appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 3-3: path appears unused. Verify use (or export if used externally).
(SC2034)
2b59448 to
7a5e0b4
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
apps/aiometadata/poster-cache-purge-handler.sh (1)
3-12: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject non-mutating requests before scheduling a purge.
Lines 3-12 parse
methodandpathbut never check them, so any request that reaches this handler schedules a purge. With/purgealready protected upstream, the remaining gap is that an authenticatedGET/HEADor browser prefetch can still clear the cache. Return405unless this is the expected purge verb, and keep the path check here as well.Proposed fix
read -r method path _ +if [ "$method" != "POST" ] || [ "$path" != "/purge" ]; then + BODY='{"success":false,"message":"method not allowed"}' + printf "HTTP/1.1 405 Method Not Allowed\r\nContent-Type: application/json\r\nContent-Length: %d\r\nConnection: close\r\n\r\n%s" "${`#BODY`}" "$BODY" + exit 0 +fi # Consume remaining headers while read -r line; do line=$(printf '%s' "$line" | tr -d '\r\n') [ -z "$line" ] && break done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/aiometadata/poster-cache-purge-handler.sh` around lines 3 - 12, The handler in poster-cache-purge-handler.sh reads method and path but never validates them, so non-mutating requests can still trigger the purge. Update the request handling logic to check the parsed method and require the expected purge verb before touching /tmp/purge-cache, and also verify the path is the purge endpoint in this script even if upstream routes it. If the method or path is not acceptable, return a 405 response instead of scheduling the cache purge.apps/aiometadata/poster-cache-nginx.conf (2)
21-25: 🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy liftRestrict
proxy_passto trusted poster origins.Line 21 derives a full upstream URL from the request path, and Line 64 forwards it verbatim. Because
apps/authelia/config/configuration.yml:842-856bypasses every non-admin path on this host, this is still a public open-proxy/SSRF surface. Only resolve approved poster CDNs here, or require signed URLs before selecting an upstream.Also applies to: 57-65
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/aiometadata/poster-cache-nginx.conf` around lines 21 - 25, The $upstream_url mapping in poster-cache-nginx.conf is turning arbitrary request paths into proxy targets, creating an open-proxy/SSRF path through proxy_pass. Update the map used by the poster cache flow so it only resolves approved poster CDN origins (or otherwise validates signed URLs) before forwarding, and ensure the proxy_pass path that consumes $upstream_url can no longer accept arbitrary upstreams.
67-72: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep relative redirects on the cache host.
Line 72 rewrites
Location: /...to$upstream_origin/..., so the client follows the redirect outsideposter-cacheand the final response never reaches this cache. Rewrite relative redirects back into the proxy URL format instead of the upstream origin.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/aiometadata/poster-cache-nginx.conf` around lines 67 - 72, The redirect handling in the proxy configuration is sending clients away from poster-cache because the proxy_redirect rule rewrites relative Location headers to $upstream_origin. Update the redirect rewrite in poster-cache-nginx.conf so relative upstream redirects are converted into absolute URLs on the cache host instead of the upstream origin, using the existing proxy_redirect directive and the upstream_origin-related rewrite block as the place to fix it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/aiometadata/compose.yaml`:
- Around line 71-85: The poster-cache container healthcheck only verifies nginx
via the /health endpoint, so it can miss failures in the background helper
processes started by the entrypoint. Update the compose service so the
healthcheck also validates the feature-dependent admin paths or process state
used by /stats.sh and the nc purge listener, or otherwise supervise those
helpers from the entrypoint; use the entrypoint command and healthcheck
definition in the poster-cache service to locate the fix.
- Around line 74-80: The poster-cache Traefik setup currently applies
authelia@docker to every request, which forces forward-auth on public poster
fetches and makes cache traffic depend on the auth stack. Update the
poster-cache labels to split routing in the compose configuration so the public
poster route bypasses Authelia while only the admin route uses authentication,
using the existing poster-cache router/service labels as the anchor for the
change.
In `@apps/aiometadata/poster-cache-stats.sh`:
- Around line 36-38: The stats payload write in poster-cache-stats.sh is not
atomic, so a concurrent read can observe a truncated or partial JSON file.
Update the write flow in the STATS_FILE generation block to first write the JSON
payload to a temporary file, then move it into place with a single rename once
the write is complete; use the existing STATS_FILE, file_count, size_human,
size_bytes, MAX_SIZE, and INACTIVE values when building the payload.
---
Duplicate comments:
In `@apps/aiometadata/poster-cache-nginx.conf`:
- Around line 21-25: The $upstream_url mapping in poster-cache-nginx.conf is
turning arbitrary request paths into proxy targets, creating an open-proxy/SSRF
path through proxy_pass. Update the map used by the poster cache flow so it only
resolves approved poster CDN origins (or otherwise validates signed URLs) before
forwarding, and ensure the proxy_pass path that consumes $upstream_url can no
longer accept arbitrary upstreams.
- Around line 67-72: The redirect handling in the proxy configuration is sending
clients away from poster-cache because the proxy_redirect rule rewrites relative
Location headers to $upstream_origin. Update the redirect rewrite in
poster-cache-nginx.conf so relative upstream redirects are converted into
absolute URLs on the cache host instead of the upstream origin, using the
existing proxy_redirect directive and the upstream_origin-related rewrite block
as the place to fix it.
In `@apps/aiometadata/poster-cache-purge-handler.sh`:
- Around line 3-12: The handler in poster-cache-purge-handler.sh reads method
and path but never validates them, so non-mutating requests can still trigger
the purge. Update the request handling logic to check the parsed method and
require the expected purge verb before touching /tmp/purge-cache, and also
verify the path is the purge endpoint in this script even if upstream routes it.
If the method or path is not acceptable, return a 405 response instead of
scheduling the cache purge.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2f157b21-ceaf-4c50-a62b-7eefb715730a
📒 Files selected for processing (9)
.envapps/aiometadata/.envapps/aiometadata/compose.yamlapps/aiometadata/poster-cache-nginx.confapps/aiometadata/poster-cache-purge-handler.shapps/aiometadata/poster-cache-stats.shapps/authelia/compose.yamlapps/authelia/config/configuration.ymlapps/cloudflare-ddns/compose.yaml
📜 Review details
🧰 Additional context used
🪛 dotenv-linter (4.0.0)
apps/aiometadata/.env
[warning] 224-224: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
🪛 Shellcheck (0.11.0)
apps/aiometadata/poster-cache-purge-handler.sh
[warning] 3-3: method appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 3-3: path appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (1)
apps/authelia/config/configuration.yml (1)
846-852: 🔒 Security & PrivacyNo change needed for these rules.
resourcesare matched against the request path, so^/purge$and^/stats$already cover these admin endpoints; query parameters would need a separatequeryrule if they ever matter.> Likely an incorrect or invalid review comment.
Add an opt-in poster-cache nginx service to the aiometadata stack that
caches poster images on disk, so repeated requests skip upstream latency
and (with cache warming) serve posters instantly. Serves /health, /stats
and /purge; disk cache lives under ${DOCKER_DATA_DIR}/poster-cache.
It sits behind Authelia with page-specific protection: the /purge and /stats
admin endpoints require two_factor, while image fetches bypass auth so Stremio
can load posters.
- poster-cache service on its own `poster-cache` profile; router carries the
authelia@docker middleware
- poster-cache-nginx.conf, poster-cache-stats.sh and poster-cache-purge-handler.sh
sidecars; the two scripts are committed executable for the nc/stats entrypoint
- Authelia: TEMPLATE_POSTER_CACHE_HOSTNAME in apps/authelia/compose.yaml plus
access_control rules in config/configuration.yml (two_factor on ^/purge$ and
^/stats$, bypass otherwise)
- POSTER_CACHE_HOSTNAME added to root .env and the cloudflare-ddns DOMAINS list
- commented opt-in POSTER_* vars in apps/aiometadata/.env (POSTER_PROXY_PREFIX_URL,
POSTER_WARMUP_URL, POSTER_WARMUP_DELAY_MS, POSTER_WARMUP_CONCURRENCY)
7a5e0b4 to
1fa57d8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/aiometadata/compose.yaml (2)
62-76: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHealth still ignores the background helper processes.
Lines 62-63 launch the purge listener and stats loop in the background, but Lines 72-76 only probe nginx’s
/healthendpoint. If either helper dies,/purgeor/statsbreaks while the container still reports healthy. This remains the same issue raised earlier.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/aiometadata/compose.yaml` around lines 62 - 76, The poster-cache healthcheck currently only verifies nginx via the /health endpoint, so it misses failures in the background helper processes started in the entrypoint. Update the health logic around the compose service entrypoint and healthcheck so it also detects whether the purge listener and stats loop launched by /purge-handler.sh and /stats.sh are still running, and fail the container health status if either helper exits instead of relying only on nginx.
67-70: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSplit the public and admin routers instead of forwarding every request to Authelia.
Line 70 still applies
authelia@dockerto the entire host. Even thoughapps/authelia/config/configuration.yml:843-852bypasses non-admin paths, Traefik will still perform a forward-auth hop on every poster fetch, so public image delivery inherits Authelia latency and outages. This remains the same issue raised earlier.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/aiometadata/compose.yaml` around lines 67 - 70, The poster-cache Traefik setup still applies authelia@docker to the whole host, so split the routing in compose.yaml into separate public and admin routers for the poster-cache service. Update the poster-cache router definition to keep public image requests on a router without forward-auth, and add a distinct admin-only router that uses authelia@docker, using the existing poster-cache router labels as the starting point.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/aiometadata/compose.yaml`:
- Line 61: The poster cache mount in the compose configuration is using the
wrong host path and should be aligned with the aiometadata data root. Update the
volume mapping in the compose file so the poster-cache storage is under the
aiometadata subtree, matching the documented persistence layout used by the rest
of the app state. Use the existing poster-cache volume entry in the compose
service as the location to fix.
In `@apps/aiometadata/poster-cache-purge-handler.sh`:
- Around line 3-12: The request handling in poster-cache-purge-handler.sh
currently schedules a purge for every proxied request after reading the request
line, so tighten the validation in the method/path parsing block before touch
/tmp/purge-cache. Use the parsed method and path variables to allow only POST
with target /purge, and for any other request return a non-success response
without creating the purge marker or sending the success body.
---
Duplicate comments:
In `@apps/aiometadata/compose.yaml`:
- Around line 62-76: The poster-cache healthcheck currently only verifies nginx
via the /health endpoint, so it misses failures in the background helper
processes started in the entrypoint. Update the health logic around the compose
service entrypoint and healthcheck so it also detects whether the purge listener
and stats loop launched by /purge-handler.sh and /stats.sh are still running,
and fail the container health status if either helper exits instead of relying
only on nginx.
- Around line 67-70: The poster-cache Traefik setup still applies
authelia@docker to the whole host, so split the routing in compose.yaml into
separate public and admin routers for the poster-cache service. Update the
poster-cache router definition to keep public image requests on a router without
forward-auth, and add a distinct admin-only router that uses authelia@docker,
using the existing poster-cache router labels as the starting point.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f6c00984-f52f-4398-9a44-4fed311bbad8
📒 Files selected for processing (9)
.envapps/aiometadata/.envapps/aiometadata/compose.yamlapps/aiometadata/poster-cache-nginx.confapps/aiometadata/poster-cache-purge-handler.shapps/aiometadata/poster-cache-stats.shapps/authelia/compose.yamlapps/authelia/config/configuration.ymlapps/cloudflare-ddns/compose.yaml
📜 Review details
🧰 Additional context used
🪛 dotenv-linter (4.0.0)
apps/aiometadata/.env
[warning] 224-224: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
🪛 Shellcheck (0.11.0)
apps/aiometadata/poster-cache-purge-handler.sh
[warning] 3-3: method appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 3-3: path appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (2)
apps/aiometadata/poster-cache-stats.sh (1)
36-38: Still writing the stats file non-atomically.
/statscan serve an empty or partial JSON document while this heredoc truncates and rewrites the file. Write to a temporary file and rename it into place.apps/authelia/config/configuration.yml (1)
845-852: 🔒 Security & PrivacyNo change needed. Authelia matches
resourcesagainst the request path only, so/purge?x=1and/stats?x=1still match^/purge$and^/stats$. The blanketbypassrule does not create a query-string bypass here.> Likely an incorrect or invalid review comment.
|
Tracked upstream in Viren070#111; same branch, so closing this one changes nothing there. |
Add an opt-in poster-cache nginx service to the aiometadata stack that caches poster images on disk, so repeated requests skip upstream latency and (with cache warming) serve posters instantly.
poster-cacheprofile, public like the other addon hosts (no authelia@docker — Stremio fetches posters through it and cannot complete a forward-auth login). Serves /health, /stats and /purge; disk cache lives under ${DOCKER_DATA_DIR}/aiometadata/poster-cacheSummary by CodeRabbit
POSTER_CACHE_HOSTNAMEand documented poster reverse-proxy cache routing/warm-up options.allprofile.