feat(ical-proxy): roll out main-4783abe (SSRF guard + /proxy concurrency cap) #1037

2026-09-27T16:10:54Z

Ghost commented

2026-09-27 16:10:54 +00:00

What changed

kubernetes/apps/ical-proxy/deployment.yaml: the pinned image goes from main-db90a59 to main-4783abe, and the comment above it is updated. Nothing else changes.

Why

REN-238 / REN-231. main-4783abe is konairius/ical-proxy main after PR #8 was merged. It also includes PR #6, which was never deployed:

#6 SSRF guard: the proxy refuses upstreams and redirect hops that resolve to private, loopback, link-local or CGNAT addresses. Redirects to public hosts are still followed, up to 5 hops. The production feed (Landkreis Amberg-Sulzbach) is a public host, so it is not affected.
#8 concurrency cap: at most 20 concurrent /proxy upstream fetches. Requests over the cap get 429 + Retry-After: 1.

Verification

Checked before merge: the main-4783abe manifest exists in the Gitea registry and can be pulled anonymously (HTTP 200, same as main-db90a59). The diff is limited to the image line and its comment.
Not yet verified: nothing has been applied to the cluster. After Flux reconciles, check that /health returns 200, that /proxy still returns the waste-calendar feed with the summary rewrite, and that both pods show 0 restarts.

Production impact

There are 2 replicas with a RollingUpdate and a readiness probe on /health, so a new pod only takes traffic once it is ready. No other workload, ingress, DNS or RBAC is touched. Blast radius: if the new build misbehaves, calendar subscriptions stop refreshing until the revert. Clients keep their cached calendars in the meantime.

Revert

Revert this commit (or set the tag back to main-db90a59). Flux then rolls the previous image back.

🤖 Generated with Claude Code

## What changed `kubernetes/apps/ical-proxy/deployment.yaml`: the pinned image goes from `main-db90a59` to `main-4783abe`, and the comment above it is updated. Nothing else changes. ## Why REN-238 / REN-231. `main-4783abe` is konairius/ical-proxy `main` after PR #8 was merged. It also includes PR #6, which was never deployed: - **#6 SSRF guard:** the proxy refuses upstreams and redirect hops that resolve to private, loopback, link-local or CGNAT addresses. Redirects to public hosts are still followed, up to 5 hops. The production feed (Landkreis Amberg-Sulzbach) is a public host, so it is not affected. - **#8 concurrency cap:** at most 20 concurrent `/proxy` upstream fetches. Requests over the cap get `429` + `Retry-After: 1`. ## Verification - **Checked before merge:** the `main-4783abe` manifest exists in the Gitea registry and can be pulled anonymously (HTTP 200, same as `main-db90a59`). The diff is limited to the image line and its comment. - **Not yet verified:** nothing has been applied to the cluster. After Flux reconciles, check that `/health` returns 200, that `/proxy` still returns the waste-calendar feed with the summary rewrite, and that both pods show 0 restarts. ## Production impact There are 2 replicas with a RollingUpdate and a readiness probe on `/health`, so a new pod only takes traffic once it is ready. No other workload, ingress, DNS or RBAC is touched. Blast radius: if the new build misbehaves, calendar subscriptions stop refreshing until the revert. Clients keep their cached calendars in the meantime. ## Revert Revert this commit (or set the tag back to `main-db90a59`). Flux then rolls the previous image back. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Ghost added 1 commit 2026-09-27 16:10:55 +00:00

feat(ical-proxy): roll out main-4783abe (SSRF guard + /proxy concurrency cap)

ci/woodpecker/pr/ci-manifests-test Pipeline was successful

Details

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

ci/woodpecker/pr/pr-review Pipeline was successful

Details

782ce2617e

Bump the pinned ical-proxy image from main-db90a59 to main-4783abe.
The new build adds two hardening changes from konairius/ical-proxy:

- #6: refuse upstreams and redirect hops that resolve to private,
  loopback, link-local or CGNAT addresses. Public redirects are still
  followed (up to 5).
- #8 (REN-231): cap concurrent /proxy upstream fetches at 20; requests
  over the cap get 429 + Retry-After instead of piling up goroutines.

Refs: REN-238, REN-231

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Ghost approved these changes

2026-09-27 16:12:13 +00:00

Ghost left a comment

Approved at 782ce2617e.

Checked:

The diff changes only the image tag and its comment in kubernetes/apps/ical-proxy/deployment.yaml (main-db90a59 -> main-4783abe).
Provenance: ical-proxy 4783abe is the merge of #8 on top of #6. Woodpecker status for 4783abe is success (pipeline 9). The registry manifest for main-4783abe exists (amd64, created 16:02:47Z, matching the pipeline).
Code diff db90a59..4783abe: an SSRF dial guard, redirect re-validation (max 5), a transport cloned from DefaultTransport with Proxy=nil, and a non-blocking 20-slot semaphore that returns 429 + Retry-After. The response body is now closed on non-200 responses too. The /health handler is unchanged.
Blast radius: 2 replicas, RollingUpdate, readiness and liveness on /health. Revert = revert this PR.
Pre-merge baseline: /health 200, /proxy with a public Google ICS 200 (462 VEVENTs).

The post-rollout check is tracked on REN-238.

Approved at 782ce2617e4868be60e62c4416d6bcbb0f26b508. Checked: - The diff changes only the image tag and its comment in kubernetes/apps/ical-proxy/deployment.yaml (main-db90a59 -> main-4783abe). - Provenance: ical-proxy 4783abe is the merge of #8 on top of #6. Woodpecker status for 4783abe is success (pipeline 9). The registry manifest for main-4783abe exists (amd64, created 16:02:47Z, matching the pipeline). - Code diff db90a59..4783abe: an SSRF dial guard, redirect re-validation (max 5), a transport cloned from DefaultTransport with Proxy=nil, and a non-blocking 20-slot semaphore that returns 429 + Retry-After. The response body is now closed on non-200 responses too. The /health handler is unchanged. - Blast radius: 2 replicas, RollingUpdate, readiness and liveness on /health. Revert = revert this PR. - Pre-merge baseline: /health 200, /proxy with a public Google ICS 200 (462 VEVENTs). The post-rollout check is tracked on REN-238.

Ghost merged commit 34b59175e2 into master

2026-09-27 16:12:14 +00:00

Ghost referenced this issue from a commit

2026-09-27 16:12:16 +00:00

Merge pull request 'feat(ical-proxy): roll out main-4783abe (SSRF guard + /proxy concurrency cap)' (#1037) from atlas/ren-238-ical-proxy-main-4783abe into master

review-bot reviewed

2026-09-27 16:14:37 +00:00

review-bot left a comment

🤖 Automated review — model CLI default (complexity: complex), commit 782ce2617e4868be60e62c4416d6bcbb0f26b508.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

Summary

This PR bumps the pinned ical-proxy image from main-db90a59 to main-4783abe in kubernetes/apps/ical-proxy/deployment.yaml and updates the accompanying comment to describe the two behavioral changes the new build carries (SSRF guard, /proxy concurrency cap). The diff is exactly what the title/body claim: one image line and its comment, 8 lines added / 6 removed, no other files touched. This matches the repo's own documented convention — ical-proxy is explicitly excluded from Flux image automation (kubernetes/infrastructure/image-automation/README.md:93-94), so a manual, reviewed tag bump like this one is the intended path. No blocking issues found.

🟡 Suggestions

The application-code claims in the comment (SSRF guard behavior, the 20-connection cap, 429 + Retry-After: 1, 5-hop redirect limit) describe code that lives in the separate konairius/ical-proxy repo, not this one. I have no way to verify those claims from this checkout — they're accepted here on the PR author's word. If that upstream PR (#8) hasn't had independent review/tests of its own, it'd be worth confirming there before leaning on this comment as documentation of guaranteed behavior.
The PR body's "Not yet verified" section (health check, feed content, pod restarts after Flux reconciles) is the real verification step for this change, since nothing about a bad build would be caught by reviewing this diff alone. Worth making sure that follow-up actually happens and isn't just aspirational text in the PR description.

Tests

Not applicable in the usual sense — this is a one-line infra pin bump with no accompanying app code in this repo to test. The meaningful "test" is the post-rollout verification described in the PR body (/health 200, feed content correct, 0 restarts); that happens after merge/reconcile, not in this diff.

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `782ce2617e4868be60e62c4416d6bcbb0f26b508`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR bumps the pinned `ical-proxy` image from `main-db90a59` to `main-4783abe` in `kubernetes/apps/ical-proxy/deployment.yaml` and updates the accompanying comment to describe the two behavioral changes the new build carries (SSRF guard, `/proxy` concurrency cap). The diff is exactly what the title/body claim: one image line and its comment, 8 lines added / 6 removed, no other files touched. This matches the repo's own documented convention — `ical-proxy` is explicitly excluded from Flux image automation (`kubernetes/infrastructure/image-automation/README.md:93-94`), so a manual, reviewed tag bump like this one is the intended path. No blocking issues found. ## 🟡 Suggestions - The application-code claims in the comment (SSRF guard behavior, the 20-connection cap, 429 + `Retry-After: 1`, 5-hop redirect limit) describe code that lives in the separate `konairius/ical-proxy` repo, not this one. I have no way to verify those claims from this checkout — they're accepted here on the PR author's word. If that upstream PR (#8) hasn't had independent review/tests of its own, it'd be worth confirming there before leaning on this comment as documentation of guaranteed behavior. - The PR body's "Not yet verified" section (health check, feed content, pod restarts after Flux reconciles) is the real verification step for this change, since nothing about a bad build would be caught by reviewing this diff alone. Worth making sure that follow-up actually happens and isn't just aspirational text in the PR description. ## Tests Not applicable in the usual sense — this is a one-line infra pin bump with no accompanying app code in this repo to test. The meaningful "test" is the post-rollout verification described in the PR body (`/health` 200, feed content correct, 0 restarts); that happens after merge/reconcile, not in this diff. ## Recommendation Looks reasonable — no blocking issues; comments above are optional.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: konairius/homelab#1037