Skip to content

test: nonce manager unit tests, docker-compose, and smoke test#3

Open
HaraldeRoessler wants to merge 3 commits into
MoltyCel:mainfrom
HaraldeRoessler:test/nonce-manager-docker-smoke
Open

test: nonce manager unit tests, docker-compose, and smoke test#3
HaraldeRoessler wants to merge 3 commits into
MoltyCel:mainfrom
HaraldeRoessler:test/nonce-manager-docker-smoke

Conversation

@HaraldeRoessler
Copy link
Copy Markdown
Contributor

Summary

  • 7 unit tests for app/nonce_manager.py (introduced in PR fix: async Web3 calls + nonce manager to prevent event loop blocking #2) covering sequential nonces, concurrent safety, reset/refetch, and per-address independence
  • docker-compose.yml for one-command local dev setup (PostgreSQL 16 + API)
  • scripts/smoke_test.sh hitting key endpoints with pass/fail reporting

How to use

Run unit tests:

pip install pytest pytest-asyncio
python -m pytest tests/test_nonce_manager.py -v

Start local dev environment:

docker compose up --build
# API at http://localhost:8000, PostgreSQL at localhost:5432

Run smoke test:

# Against local docker-compose
./scripts/smoke_test.sh http://localhost:8000 dev_test_key_local

# Against production
./scripts/smoke_test.sh https://api.moltrust.ch your_api_key

Test results (local)

All 7 nonce manager tests pass:

tests/test_nonce_manager.py::test_first_call_fetches_from_chain PASSED
tests/test_nonce_manager.py::test_second_call_increments_locally PASSED
tests/test_nonce_manager.py::test_sequential_calls_produce_unique_nonces PASSED
tests/test_nonce_manager.py::test_different_addresses_are_independent PASSED
tests/test_nonce_manager.py::test_reset_forces_refetch PASSED
tests/test_nonce_manager.py::test_reset_nonexistent_address_is_noop PASSED
tests/test_nonce_manager.py::test_concurrent_calls_get_unique_nonces PASSED

Depends on #2

Generated with Claude Code

HaraldeRoessler and others added 3 commits April 1, 2026 14:51
All synchronous Web3 RPC calls (get_transaction_count, gas_price,
send_raw_transaction, wait_for_transaction_receipt, is_connected) were
blocking the FastAPI async event loop, causing latency spikes for all
concurrent users.

Changes:
- Add app/nonce_manager.py with per-address asyncio.Lock to prevent
  nonce collisions when multiple async handlers submit transactions
  concurrently from the same wallet
- Wrap all blocking Web3 calls in asyncio.to_thread() across main.py,
  erc8004.py, and provenance/anchor.py
- Convert erc8004.py functions (post_reputation_feedback,
  register_onchain_agent, get_onchain_reputation) from sync to async
  and update all call sites with await
- Replace music VC anchoring via cast CLI subprocess with web3.py,
  eliminating private key exposure in process environment

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add tests/test_nonce_manager.py with 7 tests covering: first fetch,
  local increment, sequential uniqueness, per-address independence,
  reset/refetch, and concurrent nonce safety (20 parallel requests)
- Add docker-compose.yml for one-command local dev (PostgreSQL + API)
- Add scripts/smoke_test.sh hitting key endpoints (health, identity,
  reputation, credentials, auth enforcement)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Found during local testing:
- set -u breaks with empty arrays in bash, changed to set -eo pipefail
- ((PASS++)) returns exit code 1 when PASS=0 under set -e, use arithmetic
- Endpoint expectations adjusted to match actual API behavior
- Unique agent names per run to avoid 409 duplicate detection
- Auth test uses separate curl without valid key header
- docker-compose: add DB_HOST, DID_PRIVATE_KEY_HEX for test signing,
  env_file for optional Dilithium keys, remove obsolete version field

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@HaraldeRoessler
Copy link
Copy Markdown
Contributor Author

Merge order

Depends on #1 (needs requirements.txt and Dockerfile). Merge after #1.

Sequence: #1#3#2#4#7

Update: Smoke test and docker-compose fixed based on local testing — all 7 tests pass.

HaraldeRoessler added a commit to HaraldeRoessler/moltrust-api that referenced this pull request Apr 30, 2026
First-cut CI for moltrust-api — three lightweight jobs that catch the
most-common breakage without needing a running API stack:

  install         pip install -r requirements.txt on Python 3.12
                  (matches Dockerfile). Flags dep conflicts.
  syntax          python -m compileall on every source dir.
                  Catches SyntaxError before deploy.
  pytest-collect  pytest --collect-only on root test_*.py files.
                  Catches test ImportError without booting sandbox.

Deliberately omitted (each is a follow-up of its own):

  - lint (ruff/flake8): existing code may have legacy style issues
    that would block this PR; better added in a dedicated PR.
  - test execution: existing tests target localhost:8005 and need
    the API + Postgres + Web3 stack online. PR MoltyCel#3 adds
    docker-compose.yml + scripts/smoke_test.sh which would slot in
    as a fourth job once merged.

Closes (in part) MoltyCel#9 "RFC: CI/CD pipeline and automated testing".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HaraldeRoessler added a commit to HaraldeRoessler/moltrust-api that referenced this pull request May 12, 2026
Follow-up to commit d25e70c (SSRF). After running CodeQL default-setup
on the fork, 17 additional findings surfaced. Triage outcome:

  Already closed by earlier commits this PR:   1 (SSRF)
  False positives (dismissed via CodeQL UI):   4
  Real findings fixed in this commit:          5
  Stack-trace-exposure (deferred to design):   7

FIXES IN THIS COMMIT

  #1 [LOG SANITISATION] credit_middleware exception swallows DB password
     - app/main.py (logger.error in credit_middleware)
     `logger.error("…: %s", caller_did, e)` — the raw exception `e`
     can be an asyncpg ConnectionError whose repr() includes the
     Postgres connection string (with the password). Log only
     `type(e).__name__` instead.

  #2 [DEFENSIVE URL ENCODING] /join?ref= referrer parameter
     - app/main.py /join handler
     The redirect target is HARDCODED to https://moltrust.ch — the
     host is not user-controlled. But `f"https://moltrust.ch?ref={ref}"`
     interpolates `ref` raw, and a payload like `ref="x&malparam=…"`
     could corrupt the query string. Use `urllib.parse.quote(ref)` to
     percent-encode the value before interpolation.

  MoltyCel#3 [STDOUT TOKEN LEAK] telegram_hn_remind print(r.text)
     - scripts/telegram_hn_remind.py
     `print(f'Status: {r.status_code}, Response: {r.text}')` — if
     Telegram error responses ever echo the request URL (which contains
     the bot token in the path), the body lands in stdout / CI scrollback.
     Print only the status code.

  MoltyCel#4 [ReDoS] mpp authorization header regex
     - packages/mpp/index.js
     `auth.match(/^(?:Payment|MPP)\s+(.+)$/i)` on an unbounded header
     is polynomial-quadratic. This package is published to npm, so
     consumer servers carry the risk. Cap header at 8 KiB and use
     bounded `\s{1,8}` with a non-greedy first char.

  MoltyCel#5 [ReDoS] moltrust-openclaw-v2 base URL trim
     - moltrust-openclaw-v2/src/client.ts
     `.replace(/\/+$/, "")` is polynomial on pathological inputs.
     Replace with a `while (str.endsWith("/")) str = str.slice(0, -1)`
     loop, which is linear.

DISMISSED AS FALSE POSITIVES (no code change)

  MoltyCel#14 py/clear-text-logging-sensitive-data at SPIFFE bind log
      Logs spiffe_uri, did, caller_did — none are passwords. CodeQL
      misfires on the "did" → "id" → "password" name-similarity heuristic.

  MoltyCel#13, MoltyCel#12 py/clear-text-logging-sensitive-data in scripts/threadwatch.py
      Telegram bot token flows into the request URL but never into a
      logger or print() call — only to requests.post (which doesn't
      log URLs by default).

  MoltyCel#16 py/weak-sensitive-data-hashing in _reg_tracker
      This is in-memory rate-limit bucket-key derivation, not password
      storage. bcrypt/argon2 would be wrong here (slow + salted breaks
      the lookup). SHA-256 of the full API key is the correct primitive
      for an O(1) tracker.

EXPLICITLY DEFERRED (7 stack-trace-exposure findings)

  Multiple endpoints currently return `{"error": str(e)[:100]}` to
  callers. CodeQL flags these as info disclosure. Fixing them means
  changing the API contract — clients that parse the `error` field
  would break. This is a design call for the maintainer; deferring
  to a separate PR + discussion rather than including in this hardening
  pass.

VERIFICATION

  Python 3.12 AST parse — app/main.py + scripts/telegram_hn_remind.py
  compile cleanly. `node -c packages/mpp/index.js` clean. The TS file
  change is a syntactically-trivial loop, not type-impacting.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HaraldeRoessler added a commit to HaraldeRoessler/moltrust-api that referenced this pull request May 18, 2026
Follow-up to commit d25e70c (SSRF). After running CodeQL default-setup
on the fork, 17 additional findings surfaced. Triage outcome:

  Already closed by earlier commits this PR:   1 (SSRF)
  False positives (dismissed via CodeQL UI):   4
  Real findings fixed in this commit:          5
  Stack-trace-exposure (deferred to design):   7

FIXES IN THIS COMMIT

  #1 [LOG SANITISATION] credit_middleware exception swallows DB password
     - app/main.py (logger.error in credit_middleware)
     `logger.error("…: %s", caller_did, e)` — the raw exception `e`
     can be an asyncpg ConnectionError whose repr() includes the
     Postgres connection string (with the password). Log only
     `type(e).__name__` instead.

  #2 [DEFENSIVE URL ENCODING] /join?ref= referrer parameter
     - app/main.py /join handler
     The redirect target is HARDCODED to https://moltrust.ch — the
     host is not user-controlled. But `f"https://moltrust.ch?ref={ref}"`
     interpolates `ref` raw, and a payload like `ref="x&malparam=…"`
     could corrupt the query string. Use `urllib.parse.quote(ref)` to
     percent-encode the value before interpolation.

  MoltyCel#3 [STDOUT TOKEN LEAK] telegram_hn_remind print(r.text)
     - scripts/telegram_hn_remind.py
     `print(f'Status: {r.status_code}, Response: {r.text}')` — if
     Telegram error responses ever echo the request URL (which contains
     the bot token in the path), the body lands in stdout / CI scrollback.
     Print only the status code.

  MoltyCel#4 [ReDoS] mpp authorization header regex
     - packages/mpp/index.js
     `auth.match(/^(?:Payment|MPP)\s+(.+)$/i)` on an unbounded header
     is polynomial-quadratic. This package is published to npm, so
     consumer servers carry the risk. Cap header at 8 KiB and use
     bounded `\s{1,8}` with a non-greedy first char.

  MoltyCel#5 [ReDoS] moltrust-openclaw-v2 base URL trim
     - moltrust-openclaw-v2/src/client.ts
     `.replace(/\/+$/, "")` is polynomial on pathological inputs.
     Replace with a `while (str.endsWith("/")) str = str.slice(0, -1)`
     loop, which is linear.

DISMISSED AS FALSE POSITIVES (no code change)

  MoltyCel#14 py/clear-text-logging-sensitive-data at SPIFFE bind log
      Logs spiffe_uri, did, caller_did — none are passwords. CodeQL
      misfires on the "did" → "id" → "password" name-similarity heuristic.

  MoltyCel#13, MoltyCel#12 py/clear-text-logging-sensitive-data in scripts/threadwatch.py
      Telegram bot token flows into the request URL but never into a
      logger or print() call — only to requests.post (which doesn't
      log URLs by default).

  MoltyCel#16 py/weak-sensitive-data-hashing in _reg_tracker
      This is in-memory rate-limit bucket-key derivation, not password
      storage. bcrypt/argon2 would be wrong here (slow + salted breaks
      the lookup). SHA-256 of the full API key is the correct primitive
      for an O(1) tracker.

EXPLICITLY DEFERRED (7 stack-trace-exposure findings)

  Multiple endpoints currently return `{"error": str(e)[:100]}` to
  callers. CodeQL flags these as info disclosure. Fixing them means
  changing the API contract — clients that parse the `error` field
  would break. This is a design call for the maintainer; deferring
  to a separate PR + discussion rather than including in this hardening
  pass.

VERIFICATION

  Python 3.12 AST parse — app/main.py + scripts/telegram_hn_remind.py
  compile cleanly. `node -c packages/mpp/index.js` clean. The TS file
  change is a syntactically-trivial loop, not type-impacting.

Co-Authored-By: Claude Opus 4.7 (1M context) <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