diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 0000000..bb640e9 --- /dev/null +++ b/.dockerignore @@ -0,0 +1,32 @@ +# Everything here would otherwise be uploaded to the Docker daemon on every +# build. That matters more than usual for this repo: Fly.io builds on a remote +# builder by default, so the build context leaves the machine. +# +# The Dockerfile only COPYs Cargo.toml, Cargo.lock, src/ and config/, so +# nothing below is needed to build. + +secrets/ +.env +.env.* +!.env.example + +# The credential also lands outside secrets/: docker-compose.yml bind-mounts it +# from the context root, and the file Firebase hands you is named after the +# project. The Dockerfile COPYs none of these, but the context still travels. +firebase-service-account.json +**/*service-account*.json +**/*adminsdk*.json + +data/ +target/ +.git/ +.github/ +.claude/ +.planning/ + +*.md +docs/ +tests/ +deploy-fly.sh +docker-compose.yml +fly.toml diff --git a/.env.example b/.env.example index cbc1afd..c165640 100644 --- a/.env.example +++ b/.env.example @@ -20,7 +20,10 @@ TRUSTED_WHITELIST_ENABLED=false SERVER_PRIVATE_KEY= # Firebase Configuration (optional, for FCM support) +# The credential is not baked into the container image. Supply it at runtime +# through exactly one of the two forms below; the inline one takes precedence. FIREBASE_PROJECT_ID=mostro-test +# FIREBASE_SERVICE_ACCOUNT_JSON={"client_email":"...","private_key":"..."} FIREBASE_SERVICE_ACCOUNT_PATH=/path/to/service-account.json # UnifiedPush Configuration diff --git a/Dockerfile b/Dockerfile index b118a6b..ddf0d41 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM rust:1.90 as builder +FROM rust:1.90 AS builder WORKDIR /usr/src/app COPY Cargo.toml Cargo.lock ./ @@ -9,13 +9,41 @@ RUN cargo build --release FROM debian:bookworm-slim -RUN apt-get update && apt-get install -y \ +# curl is here only for HEALTHCHECK, which has no other way to speak HTTP in a +# slim image. It buys nothing on Fly.io, which ignores Docker health checks and +# runs the ones declared in fly.toml, but docker-compose and plain `docker run` +# rely on it. +RUN apt-get update && apt-get install -y --no-install-recommends \ ca-certificates \ + curl \ && rm -rf /var/lib/apt/lists/* +# Unprivileged runtime user. A fixed high UID and GID keep ownership predictable +# for bind mounts on the host. The group is created explicitly: `useradd --uid` +# alone picks the GID from the system range, so `USER 10001:10001` below would +# otherwise name a group that does not exist in /etc/group. +RUN groupadd --system --gid 10001 mostro \ + && useradd --system --no-create-home --shell /usr/sbin/nologin \ + --uid 10001 --gid 10001 mostro + COPY --from=builder /usr/src/app/target/release/mostro-push-backend /usr/local/bin/ -COPY secrets/ /secrets/ + +# The token store is in memory, so the only thing the process ever writes is +# the UnifiedPush endpoint file, resolved relative to the working directory. +WORKDIR /app +RUN mkdir -p /app/data && chown -R 10001:10001 /app + +# The Firebase service account is deliberately NOT copied in. Baking it into a +# layer publishes it to anyone who can pull the image, `docker save` included, +# with no need to run the container. Provide it at runtime instead, through +# FIREBASE_SERVICE_ACCOUNT_JSON or a file mounted at +# FIREBASE_SERVICE_ACCOUNT_PATH. See docs/deployment.md. ENV RUST_LOG=info +USER 10001:10001 + +HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ + CMD curl -fsS "http://127.0.0.1:${SERVER_PORT:-8080}/api/health" || exit 1 + CMD ["mostro-push-backend"] diff --git a/deploy-fly.sh b/deploy-fly.sh index 68a7563..3aa3014 100755 --- a/deploy-fly.sh +++ b/deploy-fly.sh @@ -10,9 +10,33 @@ REQUIRED_SECRETS=( NOSTR_RELAYS SERVER_PRIVATE_KEY FIREBASE_PROJECT_ID - FIREBASE_SERVICE_ACCOUNT_PATH ) +FLY_CONFIG="${FLY_CONFIG:-fly.toml}" + +# The Firebase credential no longer ships inside the image, so it must arrive at +# runtime. On Fly a secret already *is* an environment variable, so the inline +# form needs nothing else, and it is what this wrapper requires. +# +# FIREBASE_SERVICE_ACCOUNT_PATH is deliberately not accepted here. It only names +# a file, and nothing this script can read proves a file will exist there: +# `flyctl secrets list` returns names, never values, so the path the secret +# holds cannot be compared against anything fly.toml declares. A [[files]] entry +# is not evidence either — it may well write something unrelated. Guessing wrong +# means FCM starts disabled and every push is dropped in silence, which is the +# failure this check exists to catch, and it is exactly what a PATH secret left +# over from when the image carried the credential would do today. +# +# The path form stays first-class everywhere the file is genuinely under the +# operator's control — docker-compose, systemd, Kubernetes — none of which +# deploy through this script. If you do provision one on Fly via [[files]], set +# FLY_ALLOW_CREDENTIAL_PATH=1 to assert that its guest_path is the path the +# secret names. That is an assertion the operator makes, not one verified here. +CREDENTIAL_SECRETS=(FIREBASE_SERVICE_ACCOUNT_JSON) +if [[ "${FLY_ALLOW_CREDENTIAL_PATH:-}" == 1 ]]; then + CREDENTIAL_SECRETS+=(FIREBASE_SERVICE_ACCOUNT_PATH) +fi + die() { echo "Error: $*" >&2 exit 1 @@ -48,8 +72,32 @@ if (( ${#missing_secrets[@]} > 0 )); then exit 1 fi +# Without one of these the server still starts, but FCM is disabled and every +# push is silently dropped. Fail here rather than discover it in the logs. +credential_present=false +for secret in "${CREDENTIAL_SECRETS[@]}"; do + if grep -qx "${secret}" <<< "${configured_secret_names}"; then + credential_present=true + break + fi +done + +if [[ "${credential_present}" != true ]]; then + echo "No usable Firebase credential secret is set for ${APP_NAME}." >&2 + echo "Set one of:" >&2 + printf ' - %s\n' "${CREDENTIAL_SECRETS[@]}" >&2 + if (( ${#CREDENTIAL_SECRETS[@]} == 1 )); then + echo "FIREBASE_SERVICE_ACCOUNT_PATH is not accepted for Fly deploys: nothing here" >&2 + echo "can prove a file exists at the path it names. If ${FLY_CONFIG} provisions one" >&2 + echo "through [[files]], re-run with FLY_ALLOW_CREDENTIAL_PATH=1." >&2 + fi + echo "The credential is no longer baked into the image. See docs/deployment.md." >&2 + exit 1 +fi + echo "Deploying..." -flyctl deploy -a "${APP_NAME}" +# Explicit, so the config named in the messages above is the one deployed. +flyctl deploy -a "${APP_NAME}" -c "${FLY_CONFIG}" echo "Deploy complete." echo "" diff --git a/docker-compose.yml b/docker-compose.yml index 940fd26..d01ff9f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -10,17 +10,34 @@ services: - SERVER_HOST=0.0.0.0 - SERVER_PORT=8080 - FCM_ENABLED=true - # The Dockerfile bakes secrets/ into the image at /secrets. Only the - # file name is configurable: the path must stay inside the container, - # so it must not be interpolated from a host-side path. - - FIREBASE_SERVICE_ACCOUNT_PATH=/secrets/${FIREBASE_SERVICE_ACCOUNT_FILE:-firebase-service-account.json} # Opt-in: the dispatch path POSTs to the client-supplied device # token treated as a URL. Keep it false unless you accept that. - UNIFIEDPUSH_ENABLED=false - FIREBASE_PROJECT_ID=mostro-test + # The credential is not in the image. This path must match the mount + # below; without it FCM starts disabled and every push is dropped. + - FIREBASE_SERVICE_ACCOUNT_PATH=/app/secrets/firebase-service-account.json + # Bare key: forwarded from the host shell only when set there, and it + # wins over the path above. This is what makes the mount optional -- + # FIREBASE_SERVICE_ACCOUNT_JSON="$(cat firebase-service-account.json)" docker-compose up -d + # Without this line Compose never passes it in and FCM starts disabled. + - FIREBASE_SERVICE_ACCOUNT_JSON - RUST_LOG=info volumes: - # The binary runs with / as its working directory and writes the - # UnifiedPush endpoint store to data/unifiedpush_endpoints.json. - - ./data:/data + # The binary runs with /app as its working directory and writes the + # UnifiedPush endpoint store to data/unifiedpush_endpoints.json. A bind + # mount keeps the host's ownership, so ./data must be WRITABLE by UID + # 10001 (`chown 10001:10001 data`). Only matters once UnifiedPush is on. + - ./data:/app/data + # A private key: give it to UID 10001 and nobody else, rather than + # widening the mode until the container can read it. + # chmod 0600 firebase-service-account.json + # sudo chown 10001:10001 firebase-service-account.json + # Mode first: after the chown the file is UID 10001's, not yours. + # If host-side ownership is awkward, drop this mount and use the inline + # form above instead. + - ./firebase-service-account.json:/app/secrets/firebase-service-account.json:ro + # The image already declares a HEALTHCHECK, and it reads SERVER_PORT from + # the container environment. Redeclaring it here would hard-code the port + # and silently stop matching the moment SERVER_PORT above changes. restart: unless-stopped diff --git a/docs/configuration.md b/docs/configuration.md index 783a387..542f114 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -74,7 +74,8 @@ To turn the filter on/off without rebuilding, flip | `FCM_ENABLED` | `true` | Enable Firebase Cloud Messaging backend | | `UNIFIEDPUSH_ENABLED` | `false` | Enable UnifiedPush backend. Opt-in on purpose: the dispatch path POSTs to the client-supplied device token treated as a URL, so the backend stays off unless set explicitly. | | `FIREBASE_PROJECT_ID` | - | Firebase project ID, required when `FCM_ENABLED=true` | -| `FIREBASE_SERVICE_ACCOUNT_PATH` | - | Absolute path to the Firebase service-account JSON. If missing or unreadable, FCM is disabled at startup with a warning; the server keeps running. | +| `FIREBASE_SERVICE_ACCOUNT_JSON` | - | The Firebase service-account JSON itself. Takes precedence over the path form; an empty value is treated as absent. | +| `FIREBASE_SERVICE_ACCOUNT_PATH` | - | Absolute path to the Firebase service-account JSON. Used when the JSON form is unset. If neither resolves, FCM is disabled at startup with an `error` log and the server keeps running. | | `BATCH_DELAY_MS` | `5000` | Reserved (declared on `PushConfig`; not currently consumed) | | `COOLDOWN_MS` | `60000` | Reserved (declared on `PushConfig`; not currently consumed) | @@ -141,7 +142,7 @@ SERVER_PORT=8080 FCM_ENABLED=true UNIFIEDPUSH_ENABLED=false FIREBASE_PROJECT_ID=mostro-mobile -FIREBASE_SERVICE_ACCOUNT_PATH=/secrets/mostro-mobile-firebase-adminsdk.json +FIREBASE_SERVICE_ACCOUNT_PATH=/app/secrets/firebase-service-account.json # Token store TOKEN_TTL_HOURS=48 @@ -184,6 +185,12 @@ Full detail, including the known DNS-rebinding limitation, is in 1. [Firebase Console](https://console.firebase.google.com/) → your project → Project Settings → Service accounts. 2. Click **Generate new private key**, save the JSON file outside the repo. 3. Mount it into the runtime (Docker volume, Fly.io secret file, or a path on disk for systemd). -4. Set `FIREBASE_SERVICE_ACCOUNT_PATH` to the path the binary will read at startup. +4. Supply it at runtime with either `FIREBASE_SERVICE_ACCOUNT_JSON` (the JSON + itself) or `FIREBASE_SERVICE_ACCOUNT_PATH` (a path to a mounted file). It is + deliberately not baked into the container image; see + [deployment.md](./deployment.md#provisioning-the-firebase-service-account). -If FCM init fails (file missing, JSON invalid, OAuth refusal) the server logs a warning and runs without FCM. UnifiedPush, if enabled, continues to work. + The container runs as UID 10001, so a bind-mounted file must be readable by + that UID on the host. + +If FCM init fails (no credential configured, JSON invalid, OAuth refusal) the server logs at `error` and runs without FCM. UnifiedPush, if enabled, continues to work. diff --git a/docs/deployment.md b/docs/deployment.md index 13ed9b5..97c6162 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -37,7 +37,7 @@ flyctl secrets set -a mostro-push-server \ SERVER_PRIVATE_KEY="${server_private_key}" \ NOSTR_RELAYS="wss://relay.mostro.network" \ FIREBASE_PROJECT_ID="your-project-id" \ - FIREBASE_SERVICE_ACCOUNT_PATH="/secrets/firebase-service-account.json" \ + FIREBASE_SERVICE_ACCOUNT_JSON="$(cat /path/to/firebase-service-account.json)" \ FCM_ENABLED="true" \ UNIFIEDPUSH_ENABLED="false" \ SERVER_HOST="0.0.0.0" \ @@ -55,7 +55,16 @@ unset server_private_key - `NOSTR_RELAYS` - `SERVER_PRIVATE_KEY` - `FIREBASE_PROJECT_ID` -- `FIREBASE_SERVICE_ACCOUNT_PATH` +- `FIREBASE_SERVICE_ACCOUNT_JSON` + +`FIREBASE_SERVICE_ACCOUNT_PATH` is not accepted in its place. Nothing the +wrapper can read proves a file exists at the path that secret names, and a wrong +guess deploys an instance that accepts registrations and delivers nothing. If +you do provision the file through `[[files]]`, assert it explicitly: + +```bash +FLY_ALLOW_CREDENTIAL_PATH=1 ./deploy-fly.sh +``` Deploy after the secrets exist: @@ -65,7 +74,64 @@ Deploy after the secrets exist: `NOTIFY_TRUST_PROXY_HEADERS=true` is correct on Fly because requests reach the app behind the Fly edge proxy, which sets `Fly-Client-IP`. On any deployment where the app is reachable directly, leave this `false`; otherwise an attacker can rotate that header per request and defeat the per-IP limiter. -The Firebase service account JSON is bundled into the Docker image at the path specified by `FIREBASE_SERVICE_ACCOUNT_PATH`. Provision it before the build (the `Dockerfile` copies the `secrets/` directory). +### Provisioning the Firebase service account + +The credential is **not** in the image. It used to be: the `Dockerfile` copied +`secrets/` into a layer, which published the private key to anyone able to pull +the image — `docker save` and `docker history` reach it without ever running the +container. `.dockerignore` now also keeps `secrets/` out of the build context +entirely, which matters because Fly builds on a remote builder by default. + +Two ways to supply it at runtime, and exactly one is needed: + +| Variable | Use when | +|---|---| +| `FIREBASE_SERVICE_ACCOUNT_JSON` | The credential itself. Preferred on Fly.io, where a secret already *is* an environment variable. | +| `FIREBASE_SERVICE_ACCOUNT_PATH` | A path to a file mounted into the runtime. Preferred for docker-compose, systemd and Kubernetes. | + +`FIREBASE_SERVICE_ACCOUNT_JSON` takes precedence when both are set. An empty +value is treated as absent, so a half-configured deployment falls back to the +path form instead of failing. + +The inline form is the default on Fly for a specific reason: the container now +runs as UID 10001, and a file the platform mounts carries ownership and mode +this project does not control. An environment variable is readable by the +process whatever its UID. + +```bash +flyctl secrets set -a mostro-push-server \ + FIREBASE_SERVICE_ACCOUNT_JSON="$(cat /path/to/firebase-service-account.json)" +``` + +On Fly the path form is not interchangeable with the inline one, and +`deploy-fly.sh` refuses it rather than deploying an instance that cannot push. +`FIREBASE_SERVICE_ACCOUNT_PATH` only names a file; `fly.toml` as shipped +declares no `[[files]]` entry to create one, and a secret left over from when +the image carried the credential names a path that no longer exists. + +The wrapper does not try to decide the question from `fly.toml` either. It +cannot: `flyctl secrets list` returns secret names, never values, so the path +the secret holds cannot be compared against any `guest_path` declared there, and +a `[[files]]` entry may write something else entirely. Rather than accept weak +evidence, it requires the inline form and takes +`FLY_ALLOW_CREDENTIAL_PATH=1` as the operator asserting the match themselves. + +**Sequencing matters.** If no credential resolves the server still starts: +`main.rs` logs the failure and runs without FCM, because a listener and an HTTP +API without push are more useful than no server at all. The result is an +instance that accepts registrations and delivers nothing. `deploy-fly.sh` +refuses to deploy when no usable credential secret exists, but if you deploy by +other means, set the secret **before** rolling out an image built from this +Dockerfile. + +Confirm it took after the first deploy: + +```bash +flyctl logs -a mostro-push-server | grep -i "FCM service initialized" +``` + +An `FCM notifications are DISABLED` line at `error` level means the credential +did not arrive. ### Rotate `SERVER_PRIVATE_KEY` @@ -133,19 +199,40 @@ docker-compose up -d docker-compose logs -f ``` -`docker-compose.yml` points `FIREBASE_SERVICE_ACCOUNT_PATH` at `/secrets/firebase-service-account.json`, the in-image copy of `secrets/` described above. Name your service-account file accordingly before building, or keep its own name and pass it at run time: +The compose file bind-mounts `./firebase-service-account.json` to `/app/secrets/firebase-service-account.json` and points `FIREBASE_SERVICE_ACCOUNT_PATH` there. Put the credential next to `docker-compose.yml` under that name, or edit both the mount and the variable together — they have to agree, and a mismatch starts the container with FCM disabled. + +The container runs as UID 10001, so the file has to be readable by that UID on the host. Hand it to that UID rather than widening the mode — this is a private key, and `0644` would expose it to every local user: ```bash -FIREBASE_SERVICE_ACCOUNT_FILE=my-project-adminsdk.json docker-compose up -d +chmod 0600 firebase-service-account.json +sudo chown 10001:10001 firebase-service-account.json ``` -Only the file name is configurable, not the full path: the value has to resolve inside the container, so a host-side path from your `.env` must not leak into it. Verify what Compose resolved before starting: +The mode goes first on purpose: after the `chown` the file belongs to UID 10001, +and a non-root operator can no longer change it. + +Compose creates a **directory** where a bind-mount source does not exist, which surfaces later as a confusing parse failure. Confirm the file is there before the first `up`, and check what Compose resolved: ```bash -docker-compose config | grep FIREBASE_SERVICE_ACCOUNT_PATH +ls -l firebase-service-account.json +docker-compose config | grep FIREBASE_SERVICE_ACCOUNT ``` -`./data` is bind-mounted to `/data` so the UnifiedPush endpoint store survives container recreation. The binary runs with `/` as its working directory and writes the store to `data/unifiedpush_endpoints.json`, which lands at `/data/unifiedpush_endpoints.json` inside the container. +If host-side ownership is awkward, drop the mount and pass the credential inline instead. The compose file lists `FIREBASE_SERVICE_ACCOUNT_JSON` as a bare key, so a value set in the shell is forwarded into the container and takes precedence over the path: + +```bash +FIREBASE_SERVICE_ACCOUNT_JSON="$(cat firebase-service-account.json)" docker-compose up -d +``` + +Without that bare key in the `environment:` list Compose would not pass the variable in at all, and FCM would start disabled with nothing on the host to suggest why. + +`./data` is bind-mounted to `/app/data` so the UnifiedPush endpoint store survives container recreation. The image sets `WORKDIR /app` and the binary writes the store to `data/unifiedpush_endpoints.json`, which lands at `/app/data/unifiedpush_endpoints.json` inside the container. + +A bind mount keeps the host directory's ownership, overriding the one the image sets, so `./data` has to be writable by UID 10001 before enabling UnifiedPush: + +```bash +mkdir -p data && sudo chown 10001:10001 data +``` The compose file ships with `UNIFIEDPUSH_ENABLED=false` to match the binary default. The UnifiedPush dispatch path POSTs to the client-supplied device token treated as a URL, so enabling it is an explicit operator decision. @@ -230,7 +317,7 @@ The only on-disk state is `data/unifiedpush_endpoints.json`, written atomically There is no database to back up. Operationally important inputs are: -- `FIREBASE_SERVICE_ACCOUNT_PATH` JSON file (regenerate via Firebase Console if lost) +- The Firebase service account JSON, held in `FIREBASE_SERVICE_ACCOUNT_JSON` or at `FIREBASE_SERVICE_ACCOUNT_PATH` (regenerate via Firebase Console if lost) - The contents of `flyctl secrets list` (or the `.env` file on bare-metal) - `data/unifiedpush_endpoints.json` if you want UnifiedPush registrations to survive a host migration; clients will re-register on next use otherwise @@ -248,12 +335,30 @@ journalctl -u mostro-push -n 100 ### FCM not delivering ```bash -flyctl ssh console -ls -la /secrets/ # confirm the JSON is at the configured path -flyctl secrets list | grep FIREBASE # confirm path env var is set -RUST_LOG=debug flyctl deploy # redeploy with debug logging to see OAuth exchange +flyctl logs | grep -i "FCM" # startup lines: which credential loaded, and why init failed +flyctl secrets list | grep FIREBASE # confirm the credential secret exists +``` + +To see the OAuth exchange itself, raise the level on the deployed app and put it +back afterwards. `RUST_LOG` is a Fly secret, so prefixing `flyctl deploy` with it +only sets the variable for the local flyctl process and leaves the running app +at `info`: + +```bash +flyctl secrets set -a mostro-push-server RUST_LOG="debug" # restarts the machines +flyctl logs -a mostro-push-server +flyctl secrets set -a mostro-push-server RUST_LOG="info" # restore when done ``` +`FCM notifications are DISABLED` is preceded by the actual cause. Distinguish +the two cases by whether a `Loaded Firebase service account for ...` line +appears first: without it no credential resolved at all, with it the credential +arrived and was rejected downstream (bad key, OAuth refusal). + +There is no `/secrets` inside the container any more. With the inline form the +credential is an environment variable, so that startup line is the confirmation +it arrived, not a file listing. + ### `/api/notify` always 429s Either the per-IP or per-pubkey limiter is hitting. Check `Retry-After` and the response body — 429 bodies are byte-identical between the two paths, so distinguish by reproducing in isolation: diff --git a/fly.toml b/fly.toml index 998aa5e..de2fae2 100644 --- a/fly.toml +++ b/fly.toml @@ -16,6 +16,15 @@ primary_region = 'gru' auto_start_machines = true min_machines_running = 1 + # Fly ignores the Dockerfile HEALTHCHECK and runs these instead. Without one, + # a machine that is up but no longer serving stays in rotation. + [[http_service.checks]] + grace_period = '10s' + interval = '15s' + method = 'GET' + timeout = '2s' + path = '/api/health' + [[services]] protocol = 'tcp' internal_port = 8080 diff --git a/src/main.rs b/src/main.rs index 64fb601..dec1f04 100644 --- a/src/main.rs +++ b/src/main.rs @@ -106,8 +106,17 @@ async fn main() -> std::io::Result<()> { push_services.push((Arc::clone(&fcm_service) as Arc, "fcm")); } Err(e) => { - log::warn!("Failed to initialize FCM service: {}", e); - log::warn!("FCM notifications will be disabled. Set FIREBASE_SERVICE_ACCOUNT_PATH to enable."); + // error!, not warn!: the credential no longer ships inside the + // image, so a misconfigured deployment is now the likely cause + // rather than a corner case. The server keeps running — the + // Nostr listener and the HTTP API are still useful — but this + // must not scroll past as routine noise. + log::error!("Failed to initialize FCM service: {}", e); + log::error!( + "FCM notifications are DISABLED. The cause is above; the \ + credential is read from FIREBASE_SERVICE_ACCOUNT_JSON or \ + FIREBASE_SERVICE_ACCOUNT_PATH." + ); } } } diff --git a/src/push/fcm.rs b/src/push/fcm.rs index 8a9925a..53d1d68 100644 --- a/src/push/fcm.rs +++ b/src/push/fcm.rs @@ -14,6 +14,9 @@ use super::PushService; use crate::config::Config; use crate::store::Platform; +const SERVICE_ACCOUNT_JSON_ENV: &str = "FIREBASE_SERVICE_ACCOUNT_JSON"; +const SERVICE_ACCOUNT_PATH_ENV: &str = "FIREBASE_SERVICE_ACCOUNT_PATH"; + #[derive(Debug, Deserialize)] struct ServiceAccount { client_email: String, @@ -112,27 +115,13 @@ impl FcmPush { // keep call sites stable; FCM currently sources its settings from env vars. #[allow(unused_variables)] pub fn new(config: Config, client: Arc) -> Self { - let service_account_path = std::env::var("FIREBASE_SERVICE_ACCOUNT_PATH").ok(); let project_id = std::env::var("FIREBASE_PROJECT_ID").unwrap_or_else(|_| "mostro".to_string()); - let service_account = - service_account_path.and_then(|path| match fs::read_to_string(&path) { - Ok(content) => match serde_json::from_str::(&content) { - Ok(sa) => { - info!("Loaded Firebase service account for {}", sa.client_email); - Some(sa) - } - Err(e) => { - error!("Failed to parse service account JSON: {}", e); - None - } - }, - Err(e) => { - warn!("Could not read service account file {}: {}", path, e); - None - } - }); + let service_account = load_service_account( + std::env::var(SERVICE_ACCOUNT_JSON_ENV).ok(), + std::env::var(SERVICE_ACCOUNT_PATH_ENV).ok(), + ); Self { client, @@ -530,6 +519,55 @@ fn oauth_backoff(attempt: u32) -> Duration { Duration::from_millis(base + jitter) } +/// Resolves the Firebase service account, inline form first. An inline value +/// that is empty is treated as absent, so a half-set variable falls back to the +/// path rather than failing. Why the inline form exists is in +/// docs/deployment.md. +/// +/// Both are taken as arguments rather than read here so the precedence can be +/// tested without mutating process-wide environment state. +fn load_service_account(inline: Option, path: Option) -> Option { + if let Some(raw) = inline { + if !raw.trim().is_empty() { + // Named, never dumped: the value is the private key. + return parse_service_account(&raw, SERVICE_ACCOUNT_JSON_ENV); + } + warn!( + "{} is set but empty; falling back to the path form", + SERVICE_ACCOUNT_JSON_ENV + ); + } + + let path = path?; + match fs::read_to_string(&path) { + Ok(content) => parse_service_account(&content, &path), + Err(e) => { + warn!("Could not read service account file {}: {}", path, e); + None + } + } +} + +/// Parses a credential, naming only where it came from and never its content. +fn parse_service_account(raw: &str, source: &str) -> Option { + match serde_json::from_str::(raw) { + Ok(sa) => { + info!( + "Loaded Firebase service account for {} (from {})", + sa.client_email, source + ); + Some(sa) + } + Err(e) => { + error!( + "Failed to parse service account JSON from {}: {}", + source, e + ); + None + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -857,3 +895,81 @@ mod tests { ); } } + +#[cfg(test)] +mod service_account_tests { + use super::*; + + const VALID_JSON: &str = r#"{ + "client_email": "push@example.iam.gserviceaccount.com", + "private_key": "-----BEGIN PRIVATE KEY-----\nnot-a-real-key\n-----END PRIVATE KEY-----\n", + "project_id": "example-project" + }"#; + + fn temp_credential(name: &str, contents: &str) -> String { + let path = std::env::temp_dir().join(format!("mostro-sa-{name}.json")); + fs::write(&path, contents).expect("temp credential must be writable"); + path.to_string_lossy().into_owned() + } + + #[test] + fn inline_json_is_loaded() { + let sa = load_service_account(Some(VALID_JSON.to_string()), None) + .expect("inline credential must load"); + assert_eq!(sa.client_email, "push@example.iam.gserviceaccount.com"); + } + + #[test] + fn path_is_loaded_when_no_inline_value() { + let path = temp_credential("path-only", VALID_JSON); + let sa = load_service_account(None, Some(path)).expect("file credential must load"); + assert_eq!(sa.client_email, "push@example.iam.gserviceaccount.com"); + } + + /// Inline wins. On Fly a secret is an environment variable, so the inline + /// form is the one that needs no assumption about the ownership of a file + /// the platform mounts. + #[test] + fn inline_takes_precedence_over_path() { + let other = r#"{ + "client_email": "from-file@example.iam.gserviceaccount.com", + "private_key": "k", + "project_id": "p" + }"#; + let path = temp_credential("precedence", other); + + let sa = load_service_account(Some(VALID_JSON.to_string()), Some(path)) + .expect("inline credential must win"); + assert_eq!(sa.client_email, "push@example.iam.gserviceaccount.com"); + } + + /// An env var set to the empty string is a common deployment slip. Treat it + /// as absent rather than as a parse failure that masks a usable file. + #[test] + fn empty_inline_value_falls_back_to_the_path() { + let path = temp_credential("empty-inline", VALID_JSON); + + let sa = load_service_account(Some(" ".to_string()), Some(path)) + .expect("an empty inline value must not shadow the path form"); + assert_eq!(sa.client_email, "push@example.iam.gserviceaccount.com"); + } + + #[test] + fn no_credential_configured_yields_none() { + assert!(load_service_account(None, None).is_none()); + } + + #[test] + fn malformed_inline_json_yields_none() { + assert!(load_service_account(Some("{not json".to_string()), None).is_none()); + } + + #[test] + fn missing_file_yields_none() { + assert!(load_service_account( + None, + Some("/nonexistent/mostro-service-account.json".to_string()) + ) + .is_none()); + } +}