Skip to content

feat(docker): headless olympus-server bin + containerised audit harness - #1567

Merged
OlympusLedgerOrg merged 4 commits into
mainfrom
claude/docker-app-testing-892b9c
Aug 9, 2026
Merged

feat(docker): headless olympus-server bin + containerised audit harness#1567
OlympusLedgerOrg merged 4 commits into
mainfrom
claude/docker-app-testing-892b9c

Conversation

@OlympusLedgerOrg

@OlympusLedgerOrg OlympusLedgerOrg commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Summary

Adds a headless server binary and a containerised audit harness, so the full API surface can be exercised where a GTK/webkit window cannot open.

src-tauri/src/bin/olympus-server.rs boots the embedded Axum server + pg_embed Postgres without the Tauri window. Two consumers: the Linux audit/E2E container, and the buddy multi-host model where one host acts as an interim API server behind an SSH tunnel.

It is not a second architecture — it reuses the same server::start + bootstrap::run path as the desktop, taking from env (OLYMPUS_DATA_DIR, OLYMPUS_PROOFS_DIR) only what the desktop derives from Tauri. The bootstrap sequence mirrors the tauri::Builder::setup closure and must be updated alongside it.

Reviewer note — this binary skips a production gate

olympus-server does not enforce the OLYMPUS_ENV=production placeholder / ceremony-manifest exit(2) refusal, because verify_ceremony_manifests lives in the bin-only startup module and is not exported by the lib.

The re-hash-before-deserialize check in load_proving_key_with_manifest still fires on the first /zk/prove, so a tampered .ark.zkey is still rejected. But this binary is dev/test only: production should stay on the desktop binary unless that gate is lifted into the lib first. This is documented in the module header, and it is the main thing worth a decision before anyone reaches for this in a real deployment.

Also included

  • docker/Dockerfile.audit, its dockerignore, and docker/compose.audit.yml
  • tokio signal feature, for graceful shutdown (SIGTERM/ctrl-c → stop embedded PG cleanly)
  • Two CI apt-install timeouts raised 10 → 20 min. The retry loop's worst case is 3×240s update + 2×5s sleep + 300s install ≈ 17.2 min, which the old budget could not cover — so the third retry could never actually run.
  • DOCKER-APP-TESTING-2026-07-28.md, the session report

Validation

Committed through the pre-commit gate: rustfmt + cargo clippy --workspace --all-targets -- -D warnings pass. CI is the authority on the Linux/container paths, which cannot be exercised on the Windows dev host.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added a headless server mode for running Olympus independently of the desktop application.
    • Added Docker and Compose support for repeatable audit and end-to-end testing.
    • Added configurable external or embedded PostgreSQL support and graceful shutdown.
    • Added optional federation startup and readiness health checks.
  • Documentation

    • Added Docker testing results, execution instructions, validated scenarios, and remaining coverage gaps.
    • Documented 44 successful end-to-end checks, including federation, receipt verification, database roles, and lifecycle locking.

Adds `src-tauri/src/bin/olympus-server.rs`: the embedded Axum server +
pg_embed Postgres booted without the Tauri window, so the full API surface
can run where GTK/webkit cannot. Two consumers — the Linux audit/E2E
container, and the buddy multi-host model where one host acts as an interim
API server behind an SSH tunnel.

It is not a second architecture: it reuses the same server::start +
bootstrap::run path as the desktop, taking from env (OLYMPUS_DATA_DIR,
OLYMPUS_PROOFS_DIR) only what the desktop derives from Tauri. The bootstrap
sequence mirrors the tauri::Builder::setup closure and must be updated
alongside it.

NOTE — this binary does NOT enforce the OLYMPUS_ENV=production
placeholder/ceremony-manifest exit(2) gate: verify_ceremony_manifests lives
in the bin-only `startup` module and is not exported by the lib. The
re-hash-before-deserialize check in load_proving_key_with_manifest still
fires on first /zk/prove, so a tampered .ark.zkey is still rejected, but
this bin is dev/test only. Production stays on the desktop binary unless
that gate is lifted into the lib first. Documented in the module header.

Also adds the audit container (docker/Dockerfile.audit, its dockerignore,
compose.audit.yml), enables the tokio `signal` feature for graceful
shutdown, and raises two CI apt-install timeouts from 10 to 20 minutes —
the retry loop's worst case is 3x240s update + 2x5s sleep + 300s install
= ~17.2 min, which the old budget could not cover, so the third retry
could never run.

Includes the session report DOCKER-APP-TESTING-2026-07-28.md.

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

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a headless Olympus server and a Docker Compose audit harness. The harness builds an Ubuntu image, runs the server with persistent storage and proof artifacts, executes the release E2E suite, and documents test results and coverage gaps.

Changes

Docker audit harness

Layer / File(s) Summary
Headless server lifecycle
src-tauri/src/bin/olympus-server.rs, src-tauri/Cargo.toml
Adds environment-based startup, database selection, shared bootstrap, background jobs, federation initialization, HTTP readiness, and cross-platform graceful shutdown.
Audit image and build context
docker/Dockerfile.audit, docker/Dockerfile.audit.dockerignore
Adds a multi-stage Ubuntu image that builds olympus-server with federation and quorum-circuit features, installs runtime libraries, runs as olympus, and retains required ZK artifacts.
Compose service and E2E execution
docker/compose.audit.yml
Adds persistent server storage, healthchecks, proof-key mounts, optional federation settings, and a profile-gated E2E runner with bounded Cargo resources.
Harness validation and instructions
DOCKER-APP-TESTING-2026-07-28.md
Documents 44/44 E2E checks, federation and zkVM results, PostgreSQL validation, corrected test claims, coverage gaps, proof-key requirements, and execution commands.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the headless server binary and Docker audit harness added by the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Signed-off-by: Anthony Smith <olympusledgerorg@gmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (1)
docker/Dockerfile.audit (1)

46-55: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Build the audit image with the locked dependency graph.

docker/Dockerfile.audit:54 runs cargo build without --locked. If dependency resolution changes, the audit image can validate code against packages not recorded in the reviewed revision.

Add --locked and ensure the repository tracks a current Cargo.lock.

🤖 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/Dockerfile.audit` around lines 46 - 55, Update the cargo build command
in Dockerfile.audit to include the --locked flag, and ensure the repository
contains an up-to-date Cargo.lock matching the reviewed dependency graph.
🤖 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-APP-TESTING-2026-07-28.md`:
- Around line 208-218: Update the branch-state statement in the “Changes on this
branch” section to accurately indicate that the listed changes are committed,
using a date-qualified historical status if appropriate. Keep the file list
unchanged and ensure the wording is precise and auditable.
- Around line 76-78: Update the fenced code blocks around the test output and
inventory sections at the referenced locations to declare the text language,
using text after each opening fence. Apply this consistently to all three blocks
while preserving their contents.

In `@docker/compose.audit.yml`:
- Around line 38-39: Replace the committed default OLYMPUS_ADMIN_KEY values in
both affected service definitions with required Docker Compose interpolation, so
startup fails when the operator has not provided a key. Update the documented
audit compose commands to export or otherwise set OLYMPUS_ADMIN_KEY before
invocation, while preserving the existing admin-key wiring.

In `@src-tauri/src/bin/olympus-server.rs`:
- Around line 27-38: Update the headless binary startup flow around the
documented divergence and main entrypoint so OLYMPUS_ENV=production is refused
before database initialization. Reuse the existing ceremony-manifest production
gate if it can be moved into the library; otherwise add an equivalent early
environment check in olympus-server.rs that exits with the established refusal
behavior.
- Around line 226-230: Ensure every shutdown path stops embedded PostgreSQL
before exiting. In the server::start bind-failure branch, invoke stop_db()
before process::exit(2). Update the normal shutdown handling near stop_db() to
check its result and report failures instead of unconditionally claiming a clean
shutdown.
- Around line 89-93: Update the external database error construction in the
DATABASE_URL branch of the server startup flow to avoid including the full URL
or any userinfo in db_error. Use a generic connection-failure message while
preserving the existing fatal-path handling.

---

Nitpick comments:
In `@docker/Dockerfile.audit`:
- Around line 46-55: Update the cargo build command in Dockerfile.audit to
include the --locked flag, and ensure the repository contains an up-to-date
Cargo.lock matching the reviewed dependency graph.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e131ac57-610c-4f0f-8b2b-9a95e91400a4

📥 Commits

Reviewing files that changed from the base of the PR and between 985adc1 and a965978.

📒 Files selected for processing (6)
  • DOCKER-APP-TESTING-2026-07-28.md
  • docker/Dockerfile.audit
  • docker/Dockerfile.audit.dockerignore
  • docker/compose.audit.yml
  • src-tauri/Cargo.toml
  • src-tauri/src/bin/olympus-server.rs

Comment thread DOCKER-APP-TESTING-2026-07-28.md Outdated
Comment thread DOCKER-APP-TESTING-2026-07-28.md
Comment thread docker/compose.audit.yml Outdated
Comment thread src-tauri/src/bin/olympus-server.rs Outdated
Comment thread src-tauri/src/bin/olympus-server.rs
Comment thread src-tauri/src/bin/olympus-server.rs
- Require OLYMPUS_ADMIN_KEY via Compose interpolation instead of a
  committed default, in both the server and e2e service definitions.
- olympus-server: refuse to start under OLYMPUS_ENV=production before
  touching the database, since this binary skips the desktop's
  ceremony-manifest gate; stop embedded Postgres on the bind-failure
  path too, and report stop_db() failures on normal shutdown instead
  of claiming a clean stop unconditionally.
- olympus-server: drop the full DATABASE_URL (which may carry
  userinfo) from the external-DB connection error message.
- Dockerfile.audit: build with --locked.
- DOCKER-APP-TESTING doc: correct the stale "nothing is committed"
  claim to a date-qualified historical statement, tag the output
  code blocks as text, and note the new OLYMPUS_ADMIN_KEY requirement
  in the run instructions.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@OlympusLedgerOrg

Copy link
Copy Markdown
Owner Author

@coderabbitai I've pushed a fix addressing your review findings on this PR:

  • DOCKER-APP-TESTING-2026-07-28.md: corrected the stale "nothing is committed" claim to a date-qualified historical statement (files landed in 1d75974e, 2026-08-06); tagged the three untagged output code fences as text; noted the new OLYMPUS_ADMIN_KEY requirement in the run instructions.
  • docker/compose.audit.yml: replaced the committed default OLYMPUS_ADMIN_KEY in both service definitions with required ${OLYMPUS_ADMIN_KEY:?...} interpolation, so startup fails closed without an operator-supplied key.
  • src-tauri/src/bin/olympus-server.rs: added an early OLYMPUS_ENV=production refusal (exit 2, before any DB connection) since this headless binary skips the desktop's ceremony-manifest gate; the bind-failure path now stops embedded Postgres before exiting, and normal shutdown reports stop_db() failures instead of unconditionally claiming a clean stop; the external-DB error message no longer echoes the full DATABASE_URL (which may carry userinfo).
  • docker/Dockerfile.audit: added --locked to the release build (nitpick).

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== PR status and merge base =="
git status --short
base="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)"
printf 'merge-base: %s\n' "$base"

echo "== Changed files =="
git diff --name-status "$base"...HEAD

echo "== Diff summary =="
git diff --stat "$base"...HEAD

echo "== Changed-file diff =="
git diff --find-renames --find-copies --unified=80 "$base"...HEAD -- \
  DOCKER-APP-TESTING-2026-07-28.md \
  docker/Dockerfile.audit \
  docker/Dockerfile.audit.dockerignore \
  docker/compose.audit.yml \
  src-tauri/Cargo.toml \
  src-tauri/src/bin/olympus-server.rs \
  .github

echo "== Rust module map =="
ast-grep outline src-tauri/src --items all --type function,struct,enum,impl --match 'start|run|bootstrap|Config|Database|Server|shutdown|stop_db' 2>/dev/null || true

echo "== Relevant shared API references =="
rg -n -C 4 --type rust \
  'pub\s+(async\s+)?fn\s+(start|run|stop_db)\b|\b(start|run|stop_db)\s*\(' \
  src-tauri/src

echo "== Docker and Compose configuration references =="
rg -n -C 3 \
  'OLYMPUS_(ADMIN_KEY|ENV|DATA_DIR|PROOFS_DIR)|DATABASE_URL|healthcheck|build:|dockerfile:|context:|volumes:|user:' \
  docker DOCKER-APP-TESTING-2026-07-28.md

Length of output: 99929


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Current revision =="
git show -s --format='HEAD=%H%nsubject=%s%ncommitter-date=%cI' HEAD
git log --oneline --decorate -12
git remote -v

echo "== Exact changed-file line ranges =="
nl -ba src-tauri/src/bin/olympus-server.rs | sed -n '55,125p;220,305p'
nl -ba docker/compose.audit.yml | sed -n '20,125p'
nl -ba docker/Dockerfile.audit | sed -n '20,62p'
nl -ba DOCKER-APP-TESTING-2026-07-28.md | sed -n '55,92p;180,235p'

echo "== Desktop startup and ceremony gate =="
ast-grep outline src-tauri/src/main.rs --items all --type function --match 'setup|verify_ceremony|startup' || true
rg -n -C 8 --type rust \
  'verify_ceremony_manifests|OLYMPUS_ENV|init_embedded|connect_external|server::start|stop_db|confirm_and_disarm' \
  src-tauri/src/main.rs src-tauri/src/startup.rs src-tauri/src/db.rs

echo "== Bootstrap API-key persistence and recovery paths =="
rg -n -C 7 --type rust \
  'system_api_key|derive_api_key_from_bjj|freshly_generated|keychain|ensure_system_user|ensure_system_api_key' \
  src-tauri/src/bootstrap.rs src-tauri/src

echo "== Docker ignore and build-input checks =="
printf '%s\n' '-- root Docker ignore --'
if [ -f .dockerignore ]; then nl -ba .dockerignore; else echo 'no root .dockerignore'; fi
printf '%s\n' '-- Dockerfile-specific ignore existence --'
ls -l docker/Dockerfile.audit.dockerignore
printf '%s\n' '-- lockfile presence and package references --'
fd -a '^Cargo\.lock$' . -x sh -c 'echo "LOCK: $1"; grep -n "^name = \"olympus-desktop\"$" "$1" || true' sh {}

Length of output: 3226


🏁 Script executed:

#!/bin/bash
set -euo pipefail

show_lines() {
  file="$1"
  ranges="$2"
  awk -v ranges="$ranges" '
    BEGIN {
      count = split(ranges, parts, ";")
      for (i = 1; i <= count; i++) {
        split(parts[i], pair, "-")
        start[i] = pair[1]
        end[i] = pair[2]
      }
    }
    {
      for (i = 1; i <= count; i++) {
        if (FNR >= start[i] && FNR <= end[i]) {
          printf "%6d\t%s\n", FNR, $0
          break
        }
      }
    }
  ' "$file"
}

echo "== GitHub PR state and commits =="
gh pr view 1567 --repo OlympusLedgerOrg/Olympus --json headRefName,headRefOid,updatedAt,commits \
  --jq '{headRefName,headRefOid,updatedAt, commits: [.commits[] | {oid,messageHeadline,committedDate}]}'

echo "== Headless server =="
show_lines src-tauri/src/bin/olympus-server.rs '55-125;220-305'

echo "== Compose harness =="
show_lines docker/compose.audit.yml '20-125'

echo "== Dockerfile and report =="
show_lines docker/Dockerfile.audit '20-62'
show_lines DOCKER-APP-TESTING-2026-07-28.md '55-92;180-235'

echo "== Desktop startup and ceremony gate =="
if [ -f src-tauri/src/startup.rs ]; then
  rg -n -C 8 --type rust 'verify_ceremony_manifests|OLYMPUS_ENV' src-tauri/src/startup.rs
else
  echo 'src-tauri/src/startup.rs does not exist'
fi
rg -n -C 8 --type rust \
  'verify_ceremony_manifests|OLYMPUS_ENV|init_embedded|connect_external|server::start|stop_db|confirm_and_disarm' \
  src-tauri/src/main.rs src-tauri/src/db.rs

echo "== Bootstrap API-key persistence and recovery paths =="
rg -n -C 7 --type rust \
  'system_api_key|derive_api_key_from_bjj|freshly_generated|keychain|ensure_system_user|ensure_system_api_key' \
  src-tauri/src/bootstrap.rs src-tauri/src

echo "== Docker ignore and build inputs =="
if [ -f .dockerignore ]; then
  awk '{printf "%6d\t%s\n", FNR, $0}' .dockerignore
else
  echo 'no root .dockerignore'
fi
ls -l docker/Dockerfile.audit.dockerignore
fd -a '^Cargo\.lock$' . -x sh -c 'echo "LOCK: $1"; grep -n "^name = \"olympus-desktop\"$" "$1" || true' sh {}

Length of output: 128135


@OlympusLedgerOrg Full review completed against PR head a965978.

The reported follow-up changes are not present on the GitHub PR ref. The ref contains only 1d75974 and a965978. The following findings remain.

  1. High — Do not commit a usable admin key.
    docker/compose.audit.yml:39 and docker/compose.audit.yml:99 set OLYMPUS_ADMIN_KEY to audit-admin-key-change-me. Any user who can read the repository can authenticate to /admin/* on a harness started with this file. Replace both values with required interpolation such as ${OLYMPUS_ADMIN_KEY:?set OLYMPUS_ADMIN_KEY}. Update the run instructions to require the variable.

  2. High — Refuse production mode before initialization.
    src-tauri/src/bin/olympus-server.rs:61 begins initialization, and Line 89 can connect to or start PostgreSQL, without the desktop production safety gate. The desktop path rejects unsafe production configuration and ceremony-manifest failures before it serves requests. The headless binary must reject OLYMPUS_ENV=production with exit status 2 before database activity, until it can reuse the complete production gate.

  3. Medium — Do not log DATABASE_URL.
    src-tauri/src/bin/olympus-server.rs:92 formats the full URL into the startup error. PostgreSQL URLs can contain usernames and passwords. Replace the message with a credential-free connection failure message.

  4. Medium — Handle embedded PostgreSQL cleanup failures.
    src-tauri/src/bin/olympus-server.rs:228 exits after an Axum bind failure without stopping an already started embedded PostgreSQL instance. On normal shutdown, Line 288 discards stop_db() errors and Line 289 always reports a clean stop. Stop the owned instance before the bind-failure exit. On normal shutdown, report stop_db() failures and only print the clean-stop message after success.

  5. Low — Lock the container dependency resolution.
    docker/Dockerfile.audit:54 runs Cargo without --locked. Use cargo build --locked so the audit image cannot silently resolve a different dependency graph.

  6. Low — Update the session report to match the committed harness.
    DOCKER-APP-TESTING-2026-07-28.md:210 still states “Nothing is committed.” Line 212 still has an untyped output fence. The run instructions at Line 222 do not require OLYMPUS_ADMIN_KEY.

✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docker/compose.audit.yml (1)

1-129: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the failing Prettier check.

docker/compose.audit.yml:1-129 fails the repository formatting check. Run Prettier and commit the generated formatting changes.

🤖 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.audit.yml` around lines 1 - 129, Run the repository’s Prettier
formatter against docker/compose.audit.yml and apply all generated formatting
changes, preserving the compose configuration and comments; verify the
formatting check passes afterward.

Source: Pipeline failures

🤖 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/Dockerfile.audit`:
- Around line 54-55: Update the cargo build invocation in
docker/Dockerfile.audit:54-55 to include --locked, and update the cargo test
invocation in docker/compose.audit.yml:123 to include --locked, ensuring both
audit commands use the committed Cargo.lock without changing their existing
behavior.

---

Outside diff comments:
In `@docker/compose.audit.yml`:
- Around line 1-129: Run the repository’s Prettier formatter against
docker/compose.audit.yml and apply all generated formatting changes, preserving
the compose configuration and comments; verify the formatting check passes
afterward.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a411a461-9274-442c-a43a-41beb76dca78

📥 Commits

Reviewing files that changed from the base of the PR and between 985adc1 and a965978.

📒 Files selected for processing (6)
  • DOCKER-APP-TESTING-2026-07-28.md
  • docker/Dockerfile.audit
  • docker/Dockerfile.audit.dockerignore
  • docker/compose.audit.yml
  • src-tauri/Cargo.toml
  • src-tauri/src/bin/olympus-server.rs

Comment thread docker/Dockerfile.audit Outdated
Comment on lines +54 to +55
RUN cargo build --release --bin olympus-server --features federation,quorum-circuit \
&& strip target/release/olympus-server || true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## relevant files"
git ls-files | rg '(^|/)docker/(Dockerfile\.audit|compose\.audit\.yml)|(^|/)CLAUDE\.md|(^|/)Cargo\.lock$|Docker-APP-TESTING-2026-07-28\.md|DOCKER-APP-TESTING-2026-07-28\.md' || true

echo
echo "## docker/Dockerfile.audit around build"
cat -n docker/Dockerfile.audit | sed -n '1,90p'

echo
echo "## docker/compose.audit.yml around commands"
cat -n docker/compose.audit.yml | sed -n '100,135p'

echo
echo "## Cargo files and lock presence"
git ls-files | rg '(^|/)Cargo\.(toml|lock)$|\.cargo/config' | sed -n '1,120p'

echo
echo "## search cargo invocations"
rg -n "cargo (build|test|run|check|update)|--locked" docker Cargo.toml Cargo.lock .cargo 2>/dev/null | sed -n '1,220p'

Repository: OlympusLedgerOrg/Olympus

Length of output: 8856


Lock audit container Cargo resolution.

Use --locked at both audit cargo invocations so the container cannot resolve a dependency graph outside the committed Cargo.lock.

  • docker/Dockerfile.audit:54: add --locked to cargo build.
  • docker/compose.audit.yml:123: add --locked to cargo test.
📍 Affects 2 files
  • docker/Dockerfile.audit#L54-L55 (this comment)
  • docker/compose.audit.yml#L123-L123
🤖 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/Dockerfile.audit` around lines 54 - 55, Update the cargo build
invocation in docker/Dockerfile.audit:54-55 to include --locked, and update the
cargo test invocation in docker/compose.audit.yml:123 to include --locked,
ensuring both audit commands use the committed Cargo.lock without changing their
existing behavior.

@OlympusLedgerOrg

Copy link
Copy Markdown
Owner Author

@coderabbitai the fix commit (b8b64f02) is now actually on the PR ref — my earlier comment was posted before the push had landed (apologies for the noise). Re-checking each finding against b8b64f02:

  1. High — committed admin key: fixed. docker/compose.audit.yml lines 39 and 99 both now use ${OLYMPUS_ADMIN_KEY:?set OLYMPUS_ADMIN_KEY before running the audit harness}; the run instructions in DOCKER-APP-TESTING-2026-07-28.md now require exporting it first.
  2. High — production mode not refused: fixed. olympus-server.rs now checks OLYMPUS_ENV for production/prod immediately after tracing init and exits 2 before the data dir, ZK dir, or any DB connection is touched.
  3. Medium — DATABASE_URL logged: fixed. The external-DB connect failure message is now a fixed string ("Could not connect to external database; check DATABASE_URL and that it is running.") with no interpolation of the URL.
  4. Medium — embedded PostgreSQL cleanup: fixed. The bind-failure branch now calls stop_db() on the embedded instance before exit(2); normal shutdown now matches on stop_db()'s result and only prints the "stopped cleanly" line on Ok, printing a WARNING with the error otherwise.
  5. Low — Dockerfile.audit not locked: fixed. cargo build --locked --release --bin olympus-server ....
  6. Low — stale session report: fixed. "Nothing is committed" replaced with a date-qualified note pointing at 1d75974e; the output fences are tagged text; the run instructions require OLYMPUS_ADMIN_KEY.

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

@OlympusLedgerOrg I will re-review PR #1567 against the current PR head, including the six reported fixes.

⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 22 minutes.

@OlympusLedgerOrg

Copy link
Copy Markdown
Owner Author

@ coderabbit review

- prettier: reformat docker/compose.audit.yml's e2e command array
- supply-chain: bump rkyv 0.8.16 -> 0.8.18 (RUSTSEC-2026-0233/0234/0235,
  fixed transitively via wasmer in the ark-circom prover stack)
- license headers: add missing SPDX header to the new
  src-tauri/src/bin/olympus-server.rs (masked by the prettier failure
  short-circuiting the tooling:check chain before it ran)

Co-authored-by: Claude <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
docker/compose.audit.yml (2)

62-74: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Require a successful /zk/prove in the audit E2E path.

docker/compose.audit.yml:33,67,74 allows placeholder ZK artifacts before the e2e service depends on service_healthy, while src-tauri/tests/e2e_http.rs:464-481 only checks auth gating / non-prover fallback for /zk/prove. The audit can report healthy but skip the proving path that returns 503. Add an authenticated successful /zk/prove test case or a startup preflight that rejects placeholder-built artifacts.

🤖 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.audit.yml` around lines 62 - 74, Update the audit E2E flow so
it verifies a successful authenticated /zk/prove request, extending the existing
/zk/prove coverage in e2e_http.rs beyond auth-gating and fallback checks. Ensure
the test uses valid non-placeholder artifacts from the configured
OLYMPUS_PROOFS_KEYS_HOST path and fails the audit when proving returns 503;
alternatively, add startup validation in the audit compose service that rejects
placeholder artifacts before reporting healthy.

22-25: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Keep the audit images in sync with the checked-out source.

In docker/compose.audit.yml, olympus-server at lines 22-25 and e2e at lines 89-93 use build with image: ...:latest and no pull_policy. Compose can pull a registry/cached image before building, so the audit can run stale or unreviewed image contents. Set build.pull_policy: build for both services.

Proposed fix
     image: olympus-audit:latest
+    pull_policy: build

     image: olympus-audit-builder:latest
+    pull_policy: build
🤖 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.audit.yml` around lines 22 - 25, Update the build
configuration for the olympus-server and e2e services in the Compose file to set
build.pull_policy to build, ensuring both audit images are rebuilt from the
checked-out source instead of using cached or registry images.
🤖 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.audit.yml`:
- Around line 125-138: Add the --locked flag to the cargo test command in the
compose audit configuration, placing it before the -- separator, so the audit
uses the committed Cargo.lock without changing the existing package, test,
feature, or output options.

---

Outside diff comments:
In `@docker/compose.audit.yml`:
- Around line 62-74: Update the audit E2E flow so it verifies a successful
authenticated /zk/prove request, extending the existing /zk/prove coverage in
e2e_http.rs beyond auth-gating and fallback checks. Ensure the test uses valid
non-placeholder artifacts from the configured OLYMPUS_PROOFS_KEYS_HOST path and
fails the audit when proving returns 503; alternatively, add startup validation
in the audit compose service that rejects placeholder artifacts before reporting
healthy.
- Around line 22-25: Update the build configuration for the olympus-server and
e2e services in the Compose file to set build.pull_policy to build, ensuring
both audit images are rebuilt from the checked-out source instead of using
cached or registry images.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: afec2806-d056-4754-81b0-2508915242d4

📥 Commits

Reviewing files that changed from the base of the PR and between a965978 and 726b89f.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock, !**/*.lock, !**/Cargo.lock
📒 Files selected for processing (4)
  • DOCKER-APP-TESTING-2026-07-28.md
  • docker/Dockerfile.audit
  • docker/compose.audit.yml
  • src-tauri/src/bin/olympus-server.rs
🚧 Files skipped from review as they are similar to previous changes (3)
  • docker/Dockerfile.audit
  • DOCKER-APP-TESTING-2026-07-28.md
  • src-tauri/src/bin/olympus-server.rs

Comment thread docker/compose.audit.yml
Comment on lines +125 to +138
command:
[
"cargo",
"test",
"--release",
"-p",
"olympus-desktop",
"--test",
"e2e_http",
"--features",
"federation,quorum-circuit",
"--",
"--nocapture",
]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

rg -n -C 4 --glob 'Dockerfile*' --glob '*.yml' --glob '*.yaml' \
  --glob '*.toml' -- '--locked|cargo (build|test)' .

Repository: OlympusLedgerOrg/Olympus

Length of output: 7522


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

sed -n '80,150p' docker/compose.audit.yml

Repository: OlympusLedgerOrg/Olympus

Length of output: 3005


Enforce the committed Cargo lockfile.

docker/compose.audit.yml:125-138 runs cargo test without --locked, so Cargo can resolve dependencies differently from Cargo.lock instead of rejecting a mismatch. Add --locked before the test separator to keep the audit build deterministic.

Proposed fix
         "cargo",
         "test",
+        "--locked",
         "--release",
📝 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
command:
[
"cargo",
"test",
"--release",
"-p",
"olympus-desktop",
"--test",
"e2e_http",
"--features",
"federation,quorum-circuit",
"--",
"--nocapture",
]
command:
[
"cargo",
"test",
"--locked",
"--release",
"-p",
"olympus-desktop",
"--test",
"e2e_http",
"--features",
"federation,quorum-circuit",
"--",
"--nocapture",
]
🤖 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.audit.yml` around lines 125 - 138, Add the --locked flag to
the cargo test command in the compose audit configuration, placing it before the
-- separator, so the audit uses the committed Cargo.lock without changing the
existing package, test, feature, or output options.

@OlympusLedgerOrg
OlympusLedgerOrg merged commit b049204 into main Aug 9, 2026
47 of 48 checks passed
@OlympusLedgerOrg
OlympusLedgerOrg deleted the claude/docker-app-testing-892b9c branch August 9, 2026 11:59
OlympusLedgerOrg added a commit that referenced this pull request Aug 9, 2026
…1593)

- js-yaml: force the eslintrc-transitive 4.x line to 4.3.1
  (GHSA-5p4m-2wfm-xmqj, quadratic CPU on !!omap resolution)
- nanoid: force 3.3.17 (GHSA-2v37-7h3g-55p8, infinite loop with
  zero-size custom generators)

Both were masked in the prior CI run by the rkyv cargo-audit failure
short-circuiting the supply-chain job before it reached `pnpm audit`.

Co-authored-by: Claude <noreply@anthropic.com>
OlympusLedgerOrg added a commit that referenced this pull request Aug 9, 2026
Follow-up to PR #1567's CodeRabbit review: the `Dockerfile.audit` build
already uses --locked, but the e2e service's `cargo test` invocation in
compose.audit.yml did not, so the audit-image builder stage could
silently resolve a different dependency graph for the test binary than
Cargo.lock pins. All other items from that review round (admin-key
interpolation, the headless binary's production refusal gate, the
DATABASE_URL-free error message, embedded-Postgres cleanup on both
shutdown paths, and the DOCKER-APP-TESTING doc corrections) already
landed on main via the earlier follow-up commits on that PR branch —
verified present here before making this change.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant