Local development stack: infra-only compose for martyrology-api - #29
Merged
Conversation
Two compose stacks mirroring LiturgicalCalendar's: a minimal, infra-only one in martyrology-api that the host-run uvicorn talks to, and a full containerized one in martyrology-frontend that defaults to GitHub refs with a local override pointing at sibling checkouts. The full stack mirrors cdcf-infra production topology (single origin behind an nginx proxy, versions pinned to production's) because it exists to verify an OIDC flow that will run against cdcf-infra. The minimal stack deliberately diverges: it drops login v2 and the proxy, since nothing in the API repo performs an interactive sign-in. The OpenFGA model and tuples come from a clone of cdcf-infra rather than a vendored copy, so the authoritative model production uploads cannot drift. Also lands the infrastructure contract for the permission-request and notification subsystem — a martyrology database, Alembic, and an api-migrate one-shot service — without designing the subsystem itself, which gets its own spec. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fourteen tasks: eight in martyrology-api (internal-URL setting, Alembic baseline, Dockerfile, minimal compose, OpenFGA + seeder, api-migrate, provisioning scripts, smoke test), five in martyrology-frontend (Dockerfile, full compose with the single-origin proxy, app services, local-sibling override, scripts), and one in cdcf-infra un-staling Tasks 3-7 of the OIDC login-client plan, which is what this stack exists to unblock. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ntials - Pin both build stages to python:3.12.13-slim instead of the floating 3.12-slim tag, consistent with the already-pinned uv image. - Pin CRMEDR_REF/CLBDR_REF ARG defaults to the exact commit SHAs recorded by this repo's vendor/crmedr and vendor/clbdr submodules, so a rebuild can no longer silently pull different data than what Registry.load() was validated against. `git clone --branch` cannot check out an arbitrary SHA, so the clone step is replaced with init+fetch+checkout. - Document that scripts/init-db.sql's hardcoded passwords are placeholders for an ephemeral local-development volume only, since another repo's compose stack consumes this file directly. - Note why COPY src ./src in the final stage is load-bearing (the uv editable install's .pth hardcodes /app/src).
…lish The published port already followed ZITADEL_PORT, but ZITADEL_EXTERNALPORT was hardcoded to 8080, so overriding the port moved where Zitadel listened without changing what it advertised in its own discovery document/issuer, silently producing a broken issuer on any non-default port. Also documents in .env.example that the override must be free on the Windows host, not just inside WSL, when running under Docker Desktop on WSL2.
Adds openfga-migrate, openfga, and authz-seed to docker-compose.yml.
OpenFGA runs with OPENFGA_AUTHN_METHOD=preshared, which is required (not
a hardening choice): Settings.authz_enabled is false whenever the API
token is empty, so a tokenless OpenFGA would silently deny every check
while the stack reports healthy. authz-seed clones CatholicOS/cdcf-infra
and runs its setup-openfga.sh to create the Martyrology store, upload
the authorization model, and seed the 8 governed_by + 3 on_platform
structural tuples — verified at exactly 11 tuples via the HTTP API, and
idempotent on re-run ("All 11 structural tuple(s) already present").
Playground finding (empirical, not assumed): OpenFGA v1.15.1 panics at
startup ("the playground only supports authn method 'none'") when
OPENFGA_PLAYGROUND_ENABLED=true is combined with preshared auth, so the
container never becomes healthy. Since preshared auth is required here,
the Playground can never work in this stack — removed
OPENFGA_PLAYGROUND_ENABLED, the 3001 port mapping, and
OPENFGA_PLAYGROUND_PORT from .env.example, with comments recording why.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review found two defects in the task-6 brief itself: api-migrate declared both build: . and image: martyrology-api:latest, which would let this infra-only compose file silently overwrite the shared application tag that martyrology-frontend's stack pulls (Docker tags are global, not compose-project-scoped). Removed the image line per the design spec, which reserves that tag exclusively for the full stack's services. Also corrected .env.example's commented MARTYROLOGY_DATABASE_URL example to use martyrology_secure_password, matching the role scripts/init-db.sql actually creates.
- setup-stack.sh: create the provisioner's capture file at mode 600 before it can receive the one-time client secret, and remove it once parsed, instead of leaving it world-readable indefinitely; chmod 600 .env once written; rewrite set_env to route values through awk via ENVIRON instead of splicing them into a sed replacement string, where an unescaped `&` silently corrupts the line and `|` breaks the delimiter; guard the PRESHARED_KEY lookup with the same || true + explicit check its sibling lookups already have. - grant-superuser.sh: build the OpenFGA write body with jq -n instead of string interpolation, so a sub containing quotes/backslashes can't produce malformed JSON; print the exact grant/revoke effect (user, object, store) before writing and require confirmation unless --yes/-y is passed, reading the prompt from an explicitly opened /dev/tty rather than stdin (or a permission-bit check that can pass even when the device can't actually be opened). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bare `exec 2>/dev/null 3</dev/tty` used to detect a missing controlling terminal rebound both descriptors for the rest of the script, not just the open attempt. Harmless on the failure path (bash rolls back the whole redirection set when exec itself fails), but on the success path 2>/dev/null stuck permanently: "Aborted." on decline and any later curl error both vanished, so a failed grant would have looked identical to a successful one. Confine the open attempt to a subshell and do the real read as its own self-contained `< /dev/tty` redirection, so nothing leaks into the parent shell's descriptor table on either path. Verified against a real pty (Python pty.fork()): decline path shows "Aborted." on stderr, and a deliberately broken OpenFGA URL on the success path shows the curl failure on stderr too — .env was restored byte- identical afterwards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
scripts/smoke.sh asserts five bring-up invariants (Zitadel discovery, OpenFGA structural tuples, Alembic at head, API health, and anonymous redaction of restricted editions) and exits non-zero on any failure. Two corrections to the original plan, both already reflected in docker-compose.yml and .env.example: there is no OpenFGA Playground (v1.15.1 panics at startup when it's enabled alongside the preshared auth this stack requires), so the README points at curl against the OpenFGA API instead; and ZITADEL_PORT's override behavior is now documented, including the Docker-Desktop-on-WSL2 silent-failure trap.
…ites .gitignore previously excluded only the literal .env, so backup files like .env.bak.presmoke (holding the same OpenFGA token and Zitadel client secret) could slip past a future git add -A. Now .env* is ignored with .env.example re-included via a trailing negation. README's "Local development stack" prerequisites line named only Docker; scripts/smoke.sh also needs curl and jq, scripts/setup-stack.sh needs curl, jq, and git (it clones cdcf-infra on the host), and running the API itself needs python3. A missing jq previously surfaced as a bare "command not found" instead of anything actionable.
Re-derived the local-dev-stack scripts' external (non-coreutil) tool set directly: only curl, docker, git, and jq appear across smoke.sh, setup-stack.sh, and grant-superuser.sh. python3 is invoked nowhere — smoke.sh's JSON parsing runs entirely through jq — so listing it sent readers after a dependency the documented workflow never uses.
martyrology-frontend/scripts/{setup-stack,grant-superuser}.sh were
adapted from these two files and already carry a header naming this
repo as the counterpart that must change in lockstep. This repo's
copies had no reciprocal cue, so a maintainer editing here first — the
more likely entry point, as the older and primary repo — had nothing
pointing at the frontend copy needing the same fix. Adds the mirrored
header only; no behavioral change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ncation, smoke assertions - setup-stack.sh: make the OpenFGA store/model lookups non-fatal (`|| true`) so the "No Martyrology store found" guard can actually fire under set -euo pipefail, and add a short retry poll around the store lookup — authz-seed is a restart:"no" one-shot that `docker compose up -d` does not wait for, so this read can legitimately race it. - CI: extend shellcheck to scripts/*.sh (previously only scripts/deploy/*.sh), and clean up what it reports in the three local-dev-stack scripts (missing `cd ... || exit`, sourcing .env split so a narrow SC1091 disable applies). - setup-stack.sh, grant-superuser.sh: cut -d= -f2 -> -f2-, so an `openssl rand -base64 32` preshared key (which pads with `=`) isn't silently truncated when read back by these scripts. - smoke.sh: assertion 5 now captures the HTTP status code so a 403/500 from a broken redaction path is reported as a failure instead of collapsing into the "not attached" skip that only a real 404 should produce; assertion 1 now asserts issuer equality, not just presence; the ISSUER fallback honours ZITADEL_PORT; and the API_PORT reference (never defined in this repo) is replaced with the fixed host port this repo's stack actually uses. - .dockerignore: **/__pycache__ instead of scripts/__pycache__, so the deny-all actually denies every __pycache__ dir (src/, alembic/, scripts/), not just one. - docs: correct the local-development-stack design spec's now-stale claims (MARTYROLOGY_ZITADEL_INTERNAL_URL's final value, the nginx conf's actual divergence count, the resolved Playground question, and the provisioner's .env.local ZITADEL_INTERNAL_URL) — the decisions (D1-D9) still hold, only the described final state had drifted from what was actually implemented. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 50 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds a local Docker Compose stack with PostgreSQL, Zitadel, Mailpit, OpenFGA, migrations, provisioning, and smoke tests. It adds API containerization, Alembic scaffolding, internal Zitadel routing, environment settings, documentation, and repository support files. ChangesLocal development stack
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant DockerCompose
participant PostgreSQL
participant Zitadel
participant OpenFGA
participant SetupStack
participant Api
Developer->>DockerCompose: Start local infrastructure
DockerCompose->>PostgreSQL: Initialize databases
DockerCompose->>Zitadel: Start identity service
DockerCompose->>OpenFGA: Run migrations and start authorization service
Developer->>SetupStack: Provision local settings
SetupStack->>Zitadel: Create or update Martyrology configuration
SetupStack->>OpenFGA: Discover store and authorization model
SetupStack->>Developer: Update .env
Developer->>Api: Start API
Api->>Zitadel: Introspect tokens through internal URL
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
scripts/setup-stack.sh (1)
40-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCheck that
.envexists before reading it.If
.envis absent, eachgrepprints "No such file or directory" and|| truehides the cause. The script then fails at Line 47 with "OPENFGA_PRESHARED_KEY missing from .env", which names the wrong problem. An explicit existence check gives the correct instruction.♻️ Proposed guard
ENV_FILE=".env" +[[ -f "$ENV_FILE" ]] || { + echo "$ENV_FILE not found — copy .env.example to .env first" >&2 + exit 1 +} PAT_FILE="./.zitadel-data/automation-user.pat"🤖 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 `@scripts/setup-stack.sh` around lines 40 - 49, Add an explicit existence check for ENV_FILE before any grep-based configuration reads in setup-stack.sh, exiting with a clear message that .env is missing. Keep the existing OPENFGA_PRESHARED_KEY validation unchanged for cases where the file exists.scripts/smoke.sh (1)
49-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKeep the
docker compose runstderr for diagnosis.
2>/dev/nulldiscards the reason a run fails, such as a missing image or a stopped database. The failure branch then prints an empty$CUR. Capture stderr into$CURso the reported message names the cause.♻️ Proposed change
-CUR=$(docker compose run --rm --entrypoint alembic api-migrate current 2>/dev/null | tr -d '\r') +CUR=$(docker compose run --rm --entrypoint alembic api-migrate current 2>&1 | tr -d '\r')🤖 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 `@scripts/smoke.sh` around lines 49 - 55, Update the Alembic command assigned to CUR in the smoke check to capture stderr along with stdout instead of discarding it. Preserve the existing head check and ensure the bad call reports the captured docker compose run failure details through CUR.docker-compose.yml (1)
176-186: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPin the authorization seed to an immutable commit.
CDCF_INFRA_REFdefaults tomain, so identical checkouts can seed different authorization models and tuples over time. Use a commit SHA as the default in bothdocker-compose.ymland.env.example.🤖 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 `@docker-compose.yml` around lines 176 - 186, Update the CDCF_INFRA_REF default in both the docker-compose environment configuration and .env.example from the mutable main branch to a specific immutable commit SHA, while preserving the ability to override it through the environment.
🤖 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 `@docker-compose.yml`:
- Around line 27-31: Update the PostgreSQL healthcheck command to probe the TCP
server explicitly by targeting 127.0.0.1, while preserving the existing user and
healthcheck timing settings so openfga-migrate and api-migrate wait for the
final network-ready server.
In `@Dockerfile`:
- Around line 64-72: Create a non-root runtime user in the Dockerfile’s main
stage, grant it read access to /app and /data, switch to that user with USER,
and place the USER directive before the existing CMD that starts uvicorn.
In `@docs/superpowers/plans/2026-08-04-local-development-stack.md`:
- Around line 15-16: Remove the OpenFGA Playground topology from the local
development stack: delete port 3001 from the documented port list, remove the
later OPENFGA_PLAYGROUND_* environment variables and Playground port mapping,
and update the experimental verification step to reflect that no Playground is
configured. Keep the OpenFGA HTTP and gRPC endpoints unchanged.
- Around line 1981-1986: Update the martyrology-api environment configuration so
MARTYROLOGY_ZITADEL_INTERNAL_URL uses the public Zitadel origin with the
existing ZITADEL_PORT fallback instead of http://zitadel:8080. Add extra_hosts
with localhost mapped to host-gateway under the martyrology-api service,
matching the frontend service pattern, so Authenticator reaches the published
proxy.
In `@scripts/grant-superuser.sh`:
- Around line 111-112: Update the final success message in the grant-superuser
script to interpolate OP_LABEL instead of OP, while preserving the existing
tuple and user details.
- Around line 106-109: Update the curl invocation in the grant request to ensure
HTTP failures are detected on curl versions older than 7.76.0. Add a curl
version check and retain --fail-with-body when supported, or use a compatible
HTTP-status fallback that causes the script to stop before printing the success
message.
In `@scripts/smoke.sh`:
- Around line 11-16: Guard the `.env` source in the smoke script before
continuing when `set -u` is active: detect a missing or unreadable `.env`, print
a clear cause, and exit nonzero immediately. Keep the existing environment
export behavior for present files and prevent later expansion of
`MARTYROLOGY_OPENFGA_STORE_ID`.
- Around line 39-47: Update the OpenFGA smoke-test block around the
structural-tuples request to use the same object-only tuple_key shape and
page_size 100 as Authz.read_tuples, then follow each returned continuation_token
until pagination completes while accumulating tuple counts. Assert that the
total count is at least 11, preserving the existing success and failure
reporting.
---
Nitpick comments:
In `@docker-compose.yml`:
- Around line 176-186: Update the CDCF_INFRA_REF default in both the
docker-compose environment configuration and .env.example from the mutable main
branch to a specific immutable commit SHA, while preserving the ability to
override it through the environment.
In `@scripts/setup-stack.sh`:
- Around line 40-49: Add an explicit existence check for ENV_FILE before any
grep-based configuration reads in setup-stack.sh, exiting with a clear message
that .env is missing. Keep the existing OPENFGA_PRESHARED_KEY validation
unchanged for cases where the file exists.
In `@scripts/smoke.sh`:
- Around line 49-55: Update the Alembic command assigned to CUR in the smoke
check to capture stderr along with stdout instead of discarding it. Preserve the
existing head check and ensure the bad call reports the captured docker compose
run failure details through CUR.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9581557e-afe4-45a4-b930-5008f233ac0e
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
.dockerignore.env.example.github/workflows/ci.yml.gitignoreDockerfileREADME.mdalembic.inialembic/env.pyalembic/script.py.makoalembic/versions/0001_baseline.pydocker-compose.ymldocs/superpowers/plans/2026-08-04-local-development-stack.mddocs/superpowers/specs/2026-08-04-local-development-stack-design.mdpyproject.tomlscripts/grant-superuser.shscripts/init-db.sqlscripts/setup-stack.shscripts/smoke.shsrc/martyrology_api/app.pysrc/martyrology_api/auth.pysrc/martyrology_api/config.pytests/test_auth.pytests/test_config.pytests/test_migrations.py
Comment on lines
+15
to
+16
| - **Image versions are pinned to production's**, never `:latest`: `ghcr.io/zitadel/zitadel:v4.15.0`, `ghcr.io/zitadel/zitadel-login:v4.15.0`, `openfga/openfga:v1.15.1`, `postgres:17`, `nginx:alpine`, `adminer:latest`, `axllent/mailpit:latest`, `alpine:3.21`. | ||
| - **Ports reuse LiturgicalCalendar's numbers** (spec D5): Postgres `5432`, Zitadel `8080`, OpenFGA HTTP `8083`, OpenFGA gRPC `8084`, OpenFGA Playground `3001`, Adminer `8088`, Mailpit `8025`, API `8000`, frontend `3000`. Port `8081` is deliberately unused. All published ports bind `127.0.0.1` only. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the obsolete OpenFGA Playground topology.
The final design and README state that OpenFGA v1.15.1 panics when Playground and preshared authentication are both enabled. These global constraints still reserve port 3001.
Remove the Playground port from this section. Update the later OPENFGA_PLAYGROUND_* environment variables, port mapping, and experimental verification step to the resolved no-Playground design.
🤖 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 `@docs/superpowers/plans/2026-08-04-local-development-stack.md` around lines 15
- 16, Remove the OpenFGA Playground topology from the local development stack:
delete port 3001 from the documented port list, remove the later
OPENFGA_PLAYGROUND_* environment variables and Playground port mapping, and
update the experimental verification step to reflect that no Playground is
configured. Keep the OpenFGA HTTP and gRPC endpoints unchanged.
Comment on lines
+1981
to
+1986
| MARTYROLOGY_ZITADEL_ISSUER: http://localhost:${ZITADEL_PORT:-8080} | ||
| # The issuer above is the browser-facing origin and is what the `iss` | ||
| # claim carries. Inside this container `localhost` is its own loopback, | ||
| # so introspection is sent over the docker network instead. Transport | ||
| # only — it never affects auth_enabled. | ||
| MARTYROLOGY_ZITADEL_INTERNAL_URL: http://zitadel:8080 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Route API introspection through the public Zitadel origin.
Line 1986 sends introspection directly to http://zitadel:8080. Authenticator uses that authority for its HTTP request. The design documents that Zitadel rejects Host: zitadel:8080, so authenticated API requests will fail with 401.
Set MARTYROLOGY_ZITADEL_INTERNAL_URL to http://localhost:${ZITADEL_PORT:-8080}. Add extra_hosts: ["localhost:host-gateway"] to martyrology-api, as the frontend service already does, so the API container reaches the published proxy.
Proposed update
- MARTYROLOGY_ZITADEL_INTERNAL_URL: http://zitadel:8080
+ MARTYROLOGY_ZITADEL_INTERNAL_URL: http://localhost:${ZITADEL_PORT:-8080}
+ extra_hosts:
+ - "localhost:host-gateway"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| MARTYROLOGY_ZITADEL_ISSUER: http://localhost:${ZITADEL_PORT:-8080} | |
| # The issuer above is the browser-facing origin and is what the `iss` | |
| # claim carries. Inside this container `localhost` is its own loopback, | |
| # so introspection is sent over the docker network instead. Transport | |
| # only — it never affects auth_enabled. | |
| MARTYROLOGY_ZITADEL_INTERNAL_URL: http://zitadel:8080 | |
| MARTYROLOGY_ZITADEL_ISSUER: http://localhost:${ZITADEL_PORT:-8080} | |
| # The issuer above is the browser-facing origin and is what the `iss` | |
| # claim carries. Inside this container `localhost` is its own loopback, | |
| # so introspection is sent over the docker network instead. Transport | |
| # only — it never affects auth_enabled. | |
| MARTYROLOGY_ZITADEL_INTERNAL_URL: http://localhost:${ZITADEL_PORT:-8080} | |
| extra_hosts: | |
| - "localhost:host-gateway" |
🤖 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 `@docs/superpowers/plans/2026-08-04-local-development-stack.md` around lines
1981 - 1986, Update the martyrology-api environment configuration so
MARTYROLOGY_ZITADEL_INTERNAL_URL uses the public Zitadel origin with the
existing ZITADEL_PORT fallback instead of http://zitadel:8080. Add extra_hosts
with localhost mapped to host-gateway under the martyrology-api service,
matching the frontend service pattern, so Authenticator reaches the published
proxy.
Comment on lines
+106
to
+109
| curl -sS --fail-with-body -X POST "$API_URL/stores/$STORE_ID/write" \ | ||
| -H "Authorization: Bearer $TOKEN" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d "$BODY" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
curl --fail-with-body option introduced in which version
💡 Result:
The curl --fail-with-body command-line option was introduced in curl version 7.76.0 [1][2]. This option allows curl to return an error (exit code 22) for HTTP server errors (400 or greater) while still outputting and saving the response body, unlike the original --fail option which suppresses the body [3][2].
Citations:
- 1: https://curl.se/docs/optionsall.html
- 2: https://daniel.haxx.se/blog/2021/02/11/curl-fail-with-body/
- 3: https://curl.se/docs/manpage.html
🌐 Web query:
curl 7.76.0 release notes https://curl.se/changes.html
💡 Result:
The curl 7.76.0 release, published on March 31, 2021, introduced several new features, improvements, and security fixes [1][2][3]. Key changes and features included: - New command-line options: Added support for multiple -b parameters to specify multiple cookies [1][3], and introduced --fail-with-body, which saves the response body even when an HTTP error code (400 or higher) occurs [1][3]. - TLS and security: Added an initial implementation of the rustls backend [1][3] and new DoH options to disable SSL verification specifically for DoH servers [1][3]. - Mail support: Added support for SCRAM-SHA-1 and SCRAM-SHA-256 via libgsasl [1][3]. - HTTP: Added support for reading and storing the Referer header via CURLINFO_REFERER and the --write-out '%{referer}' option [1][3]. The release also addressed two security vulnerabilities: - CVE-2021-22876: Fixed to prevent credentials from being included in the auto-referer header field [1][4]. - CVE-2021-22890: Added an 'isproxy' argument to Curl_ssl_get/addsessionid [1][4]. Full details can be found in the official changelog at https://curl.se/ch/7.76.0.html [1].
Citations:
- 1: https://curl.se/ch/7.76.0.html
- 2: https://github.com/curl/curl/releases/tag/curl-7_76_0
- 3: https://daniel.haxx.se/blog/2021/03/31/curl-7-76-0-adds-rustls/
- 4: https://www.mail-archive.com/curl-library@cool.haxx.se/msg31400.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find script and inspect relevant lines plus curl usage/version requirements"
if [ -x /bin/ls ]; then ls -l scripts/grant-superuser.sh 2>/dev/null || true; fi
wc -l scripts/grant-superuser.sh 2>/dev/null || true
sed -n '1,150p' scripts/grant-superuser.sh 2>/dev/null || true
echo
echo "Search for curl version requirement in repository"
rg -n --fixed-strings 'curl' --glob '!*/.git/**' . | rg -n 'version|curl|fail-with-body|7\.' | head -n 80 || trueRepository: CatholicOS/martyrology-api
Length of output: 13799
Require curl --fail-with-body or use a compatible fallback.
--fail-with-body requires curl 7.76.0+. This script documents curl but not a version, so older hosts run curl -X POST ... and then print “✓ grant” even after an HTTP error. Add a curl/version check or fall back to -w '%{http_code}' with a status check.
🤖 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 `@scripts/grant-superuser.sh` around lines 106 - 109, Update the curl
invocation in the grant request to ensure HTTP failures are detected on curl
versions older than 7.76.0. Add a curl version check and retain --fail-with-body
when supported, or use a compatible HTTP-status fallback that causes the script
to stop before printing the success message.
# Conflicts: # uv.lock
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- docker-compose.yml: force the Postgres healthcheck onto TCP (-h 127.0.0.1). pg_isready with no -h defaults to the UNIX socket, which the entrypoint's temp init server (listen_addresses='') also accepts on, so a socket-only check can report healthy before openfga-migrate/api-migrate/zitadel can actually reach db:5432 over the network. - Dockerfile: run the main stage as a non-root user, matching the sibling martyrology-frontend fix. Confirmed no runtime write requirement in the default config (registry reads are read-only, Alembic writes only to Postgres, LocalGitBackend's write path is opt-in and unset by default). - scripts/grant-superuser.sh: fix success message to use $OP_LABEL (grant/revoke) instead of $OP (writes/deletes). - scripts/smoke.sh: fail fast with a clear message when .env is missing instead of surfacing as an unbound-variable error later; rewrite the structural-tuple read to paginate like Authz.read_tuples (page_size 100, follow continuation_token) and assert >= 11 tuples instead of exactly 11, since grant-superuser.sh's documented workflow adds a 12th; capture stderr from the alembic-current check so failures report their cause. - scripts/setup-stack.sh: fail fast if .env doesn't exist before the grep-based reads. - docs/superpowers/plans/2026-08-04-local-development-stack.md: add dated correction notes where the plan still describes the dropped OpenFGA Playground and the wrong MARTYROLOGY_ZITADEL_INTERNAL_URL value, pointing at the design spec's resolution rather than rewriting the historical plan. --fail-with-body's curl>=7.76 requirement and pinning CDCF_INFRA_REF to a SHA were both investigated and declined: an unrecognized curl flag exits nonzero immediately under set -e (no silent bypass), and CDCF_INFRA_REF is deliberately floating so the dev stack tracks cdcf-infra's authoritative model rather than a frozen copy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…fix) CodeRabbit was rate-limited on this repo's PR #29, but the same pagination logic landed here in the same round and has the same defect: the read loop could exhaust its 10-iteration cap with TOKEN still non-empty (more pages existed) and still report the partial COUNT as trustworthy. Reuse the existing COUNT="" "unknown" sentinel in that case rather than letting a partial sum satisfy >= 11. The Login V2 status-check finding from the same round does not apply here — this script has no such check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/smoke.sh`:
- Around line 64-66: Add bounded connection and total request timeouts to the
curl invocation assigning PAGE in the smoke-check request, using
--connect-timeout and --max-time with appropriate finite values. Preserve the
existing -s/-f behavior and the failure branch that clears COUNT and breaks when
curl exits non-zero.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f9feb2d1-2c82-4414-8bac-047b63734de4
📒 Files selected for processing (6)
Dockerfiledocker-compose.ymldocs/superpowers/plans/2026-08-04-local-development-stack.mdscripts/grant-superuser.shscripts/setup-stack.shscripts/smoke.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- scripts/grant-superuser.sh
- Dockerfile
- docker-compose.yml
- scripts/setup-stack.sh
CodeRabbit flagged one unbounded curl (the paginated OpenFGA read in smoke.sh); auditing found all nine curl invocations across smoke.sh, setup-stack.sh and grant-superuser.sh were unbounded, unlike the sibling martyrology-frontend copies which already received this hardening. Mirrors the sibling's CURL_TIMEOUT=(--connect-timeout 5 --max-time 15) array pattern in the two multi-call scripts, and the sibling's inline --connect-timeout 5 --max-time 15 in grant-superuser.sh's single privileged write. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements Tasks 1-8 of
docs/superpowers/plans/2026-08-04-local-development-stack.md, from the design indocs/superpowers/specs/2026-08-04-local-development-stack-design.md.Merge this first.
martyrology-frontend's stack buildsmartyrology-api:latestfrom this repo, so its GitHub-default path only works once the Dockerfile is onmain.What this adds
An infra-only compose stack — Postgres, Zitadel, Mailpit, Adminer, OpenFGA (+ seeder), and an Alembic runner — for a host-run
uvicorn. The API is deliberately not a service here, mirroring how LiturgicalCalendarAPI keeps its own API on the host. Plus aDockerfileconsumed by the sibling repo's stack, and provisioning/smoke scripts.One code change:
MARTYROLOGY_ZITADEL_INTERNAL_URL(empty default → falls back to the issuer), used for introspection transport only. It never influencesauth_enabled, which still keys offzitadel_issueralone.Why it exists
Two things were verifiable only against production:
cdcf-infra's OIDC login-client plan were flagged stale, awaiting a local stack..envhad to setMARTYROLOGY_RESTRICTED_EDITIONS=to stay usable, because with no local authz the licensing path failed closed — so redaction was never exercised outside production.Both are resolved. That workaround is now retired.
Verified running
pytest316 passed;ruff,pyrightclean.scripts/smoke.sh: 5 passed / 0 failed / 0 skipped, exit 0.platform:martyrologysuperuser →on_platform→admin→editor→reader→can_read_textsonedition:martyrologium_romanum_2004returned{"allowed":true}, then{"allowed":false}after revoke, store back to exactly 11 structural tuples. That path was previously reasoned but untested.access: restricted-textswithtext: nullthrough real OpenFGA.Notes for review
Settings.authz_enabledrequires a non-empty token, so a tokenless OpenFGA would deny every check while the stack reported healthy.curl.Known follow-ups (non-blocking)
scripts/deploy/setup-vps-deploy-user.sh:270still usescut -d= -f2rather than-f2-. Pre-existing, reads a port number, cannot truncate in practice.alembic/sits outside bothruff's explicit paths and[tool.pyright].include; extend both when the first real model lands.path_separatorDeprecationWarning.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Bug Fixes