Skip to content

Redact Signing Server access and operational logs - #2

Open
antuz123 wants to merge 2 commits into
mainfrom
codex/r3-redacted-error-logs
Open

antuz123 wants to merge 2 commits into
mainfrom
codex/r3-redacted-error-logs

Conversation

@antuz123

@antuz123 antuz123 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Raw route, validation, process, cron, notification and delayed-signing errors could write exception contents or user/configuration identifiers into logs. This records fixed failure categories and removes inheritance request IDs and database access identifiers from operational messages. Access logs retain method, status, response size and elapsed time.

HTTP statuses and the mobile { err } response contract stay compatible. README records the remaining public-error-contract and private hosted-log review boundaries.

Validation: Node 22 compile; 112/112 tests including route/validation canaries, six failed scheduled jobs and notification provider failures; current-source and fetched-history Gitleaks scans. This is a source-only logging change with no production deployment or credential rotation. Dependency remediation remains in PR #1.

Combined local validation (9 Oct 2026): isolated merge of #1 (3cc7728c) and #2 (73399398) passed Node 22 npm ci --ignore-scripts, compile and 125/125 tests across 6 suites. Disposable SES loopback serialization, Google Secret Manager provider mock and access-log canary passed; no external provider call or real credential was used. Combined runtime audit: 0 critical/high/moderate and 1 low AWS SDK v2 advisory; current-source Gitleaks passed. Hosted provider/log acceptance still requires non-production SES IAM/verified identity/region, GCP staging project and Secret Manager access, and private running artifact/log/config visibility.

Cloud continuation (12 October 2026): this source head is unchanged and was approved by Utkarsh on 9 October. Exact-head GitHub source/test checks remain successful. An isolated unpushed integration of #1/#2/#5 at 8de93e58358d0f9066c4ccda27fbeac474c9279d passed Node 22.23.3 compile and 127/127 tests across six suites. This is compatibility evidence only, not a deployment or production route/log/provider attestation. Mac-native testing is not needed for backend source review; staging/operator and app/hardware integration acceptance remain open.

@antuz123
antuz123 requested review from cakesoft-utkarsh and a balanced review from Copilot October 7, 2026 12:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The narrowly scoped logging changes preserve existing behavior, with no blocking issues identified.

Review effort: Balanced
Findings: None

What changed in this PR

Reduces sensitive data exposure in Signing Server’s central logs while preserving HTTP responses and process behavior.

Changes:

  • Replaces exception details with fixed failure messages.
  • Removes URLs, referrers, and user agents from access logs.
  • Adds canary tests for log redaction and unchanged responses.
File Description
tests/​unit/​errorLogging.test.ts Tests redaction and response compatibility.
src/​utilities/​logger.ts Uses fixed process exception messages.
src/​middleware/​validation.ts Redacts validation exception logs.
src/​middleware/​errorHandling.ts Redacts route exception logs and updates documentation.
src/​index.ts Limits access logs to basic request metrics.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@antuz123 antuz123 changed the title Redact central Signing Server error logs Redact Signing Server access and operational logs Oct 7, 2026
@antuz123
antuz123 marked this pull request as ready for review October 7, 2026 13:21

@antuz123 antuz123 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Logging review at 7339939: I inspected the source log sinks and confirmed the access format excludes request URLs, referrers, and user agents; route, process, notification, cron, delayed-work, and database-configuration messages use fixed categories. Exact-head GitHub CI compiled and passed 112/112 tests (run 37626412363), and current-source plus fetched-history scan jobs passed (run 37626412300). A fresh local current-source Gitleaks check also found no leaks. The existing HTTP error response contract is intentionally preserved here; route-specific response hardening and mobile compatibility checks remain separate. This branch also needs reviewed dependency PR #1 before final contributor handoff, followed by private hosted log/configuration review. These checks do not establish what a deployed service logged, and this comment is not independent maintainer approval or deployment authorization.

Copy link
Copy Markdown
Contributor Author

R4 backend acceptance handoff — 12 October 2026

State: BLOCKED_ON_STAGING_OPERATOR_ACCEPTANCE
Scope: Existing R4 source only. New Assisted Multisig remains pending Adam.

Source/compatibility evidence: Public Relay runtime pin ee935359f22ff7932ea0a62e9084d4b49801f4aa passed Node 22 compile, 105/105 baseline/local-mode and 104/104 Mongo 7 replica-set backup tests during this continuation. SigningServer #1 3cc7728, #2 7339939 and #5 7c14b6e merged without conflicts into isolated unpushed verification SHA 8de93e58358d0f9066c4ccda27fbeac474c9279d; Node 22.23.3 compile and 127/127 tests in 6 suites passed. All three source PRs and Relay #3 have Utkarsh's 9 October approvals on the pinned heads; hosted source/test checks on those unchanged heads are green. These are source/local-fixture results, not a deployed-service claim.

Operator/staging acceptance still required:

  • Attest running source/artifact identity, effective route authorization and environment, dependency inventory, provider roles and log/access visibility using an approved private read-only process. Publish only non-sensitive SHA/build IDs and aggregate outcome categories.
  • On a disposable staging replica set, attest transaction support and the actual account-scoped index; exercise readback/conditional repair, two writes from one revision, delete/recreate and A→B→A generation invalidation, cross-account isolation, malformed IDs, signer-map cleanup, archive/unlink and old-client walletObject/signersObjects aliases. Ambiguous old/current field pairs must reject before mutation; stale writes must preserve concurrent data.
  • Verify synthetic old-client restore through mobile and hosted HTTP; the current contributor stack pin is not proof of production route parity. Use an independent contributor's own fork/identity to demonstrate clean setup and a scoped PR; never share a maintainer credential.
  • For SigningServer migration, use synthetic V2 fixtures via staged HTTP with real route controls: unauthorized/invalid-token requests rejected, first migration and retry preserve migrationSuccessful/setup metadata and V2 policy/restrictions, outbound responses omit verifier, migrated scalar TOTP validates, retry leaves one V3/map.
  • Inventory potentially malformed preexisting V3 records privately and obtain a separate reviewed repair decision. Redact legacy migration verifier and preserve V2 TOTP in V3 #5 leaves existing V3 records untouched.
  • Verify staged SES and Google provider success/failure with scoped non-production roles; fixed operational log categories and method/status/size/duration access logs must omit request URLs, IDs, raw exceptions and secret/config values. AWS SDK v2 migration remains separate.
  • Keep source approval, hosted CI, staging acceptance, deployment approval and running-artifact postflight as separate decisions. No deployment, credential change, owner backup mutation, production index action or funded operation has been authorized by this handoff.

Native Mac testing is not required to review backend source itself. The final app/Desktop Local handoff must separately record iOS/Android/physical signer round trips against explicitly identified compatible services.

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.

3 participants