docs: add security policy - #182
Conversation
…lines Establishes SECURITY.md with coordinated disclosure process, supported versions table (0.16.x only), and private reporting via security@mostro.network. Defines in-scope threats (mnemonic exposure, key derivation flaws, message validation bypasses, insecure permissions) vs. out-of-scope (daemon bugs, relay DoS, physical access). Includes response targets (72h acknowledgement, 7d assessment, 30d fix), 90-day disclosure window
WalkthroughAdded ChangesSecurity Policy
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b82dc5300
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Please do not disclose the issue publicly before a fix is released, unless we have | ||
| agreed otherwise. |
There was a problem hiding this comment.
When a confirmed issue remains unfixed after 90 days, step 4 permits public disclosure at the deadline, but this sentence still prohibits disclosure until a fix is released unless a separate agreement is reached. Reporters therefore cannot determine which condition governs; make the request explicitly preserve the stated 90-day limit.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
SECURITY.md (1)
21-29: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winProvide an encrypted reporting option and require secret redaction.
The policy requests proof-of-concept code, logs, and configuration but does not require reporters to remove mnemonics,
ADMIN_NSEC, tokens, or other secrets. It also provides no PGP key or secure upload path. Add an encrypted channel and require redaction or revocation of live credentials before submission.🤖 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 `@SECURITY.md` around lines 21 - 29, Update the vulnerability reporting guidance in SECURITY.md to provide an encrypted submission channel, such as a maintained PGP public key or secure upload path, alongside security@mostro.network. Add an explicit requirement that reporters redact mnemonics, ADMIN_NSEC, tokens, and other secrets from proof-of-concept code, logs, and configuration, or revoke any live credentials before submission.
🤖 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 `@SECURITY.md`:
- Around line 46-55: Update the coordinated disclosure policy in SECURITY.md so
the 90-day milestone triggers a coordinated, risk-based disclosure review rather
than automatic public disclosure. Make clear that unresolved critical issues,
especially those affecting key material or funds, must not be disclosed publicly
without an available fix or explicit agreement, and align the reporter guidance
with this rule.
- Around line 61-62: Update the key-material scope in SECURITY.md to explicitly
include ADMIN_NSEC alongside the mnemonic, identity key, and derived trade keys,
preserving the existing scope wording and coverage.
- Around line 63-64: Update the SECURITY.md scope list to remove NIP-06 and
NIP-98 from the encryption, signing, or Nostr message validation entry, keeping
NIP-44 and NIP-59 there. Add separate scope entries identifying NIP-06 as
mnemonic-based key derivation and NIP-98 as HTTP authentication with signed
events.
---
Nitpick comments:
In `@SECURITY.md`:
- Around line 21-29: Update the vulnerability reporting guidance in SECURITY.md
to provide an encrypted submission channel, such as a maintained PGP public key
or secure upload path, alongside security@mostro.network. Add an explicit
requirement that reporters redact mnemonics, ADMIN_NSEC, tokens, and other
secrets from proof-of-concept code, logs, and configuration, or revoke any live
credentials before submission.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| We follow coordinated disclosure: | ||
|
|
||
| 1. You report the issue privately by email. | ||
| 2. We confirm the issue and work on a fix. | ||
| 3. A patched release is published and users are notified. | ||
| 4. Details are disclosed publicly once a fix is available, or after 90 days from the | ||
| initial report, whichever comes first. | ||
|
|
||
| Please do not disclose the issue publicly before a fix is released, unless we have | ||
| agreed otherwise. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Make 90-day disclosure conditional for unresolved critical issues.
Line 51 permits public disclosure after 90 days even when no fix exists. Line 54 asks reporters not to disclose before a fix. These rules conflict, and publishing details of an unfixed key-material or funds vulnerability can expose users.
Define the 90-day point as a coordinated, risk-based disclosure review. Do not automatically publish details for unresolved critical issues.
🤖 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 `@SECURITY.md` around lines 46 - 55, Update the coordinated disclosure policy
in SECURITY.md so the 90-day milestone triggers a coordinated, risk-based
disclosure review rather than automatic public disclosure. Make clear that
unresolved critical issues, especially those affecting key material or funds,
must not be disclosed publicly without an available fix or explicit agreement,
and align the reporter guidance with this rule.
| - Exposure or leakage of the mnemonic, identity key or derived trade keys. | ||
| - Weaknesses in key derivation, storage or database encryption. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Include ADMIN_NSEC in the key-material scope.
README.md documents ADMIN_NSEC as a separate private key for admin and solver commands. Line 87 also states that it grants administrative capabilities. The scope currently lists only the mnemonic, identity key, and derived trade keys.
Add ADMIN_NSEC explicitly so client-side leaks are not treated as out of scope.
Proposed wording
-- Exposure or leakage of the mnemonic, identity key or derived trade keys.
+- Exposure or leakage of the mnemonic, identity key, derived trade keys, or `ADMIN_NSEC`.📝 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.
| - Exposure or leakage of the mnemonic, identity key or derived trade keys. | |
| - Weaknesses in key derivation, storage or database encryption. | |
| - Exposure or leakage of the mnemonic, identity key, derived trade keys, or `ADMIN_NSEC`. | |
| - Weaknesses in key derivation, storage or database encryption. |
🤖 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 `@SECURITY.md` around lines 61 - 62, Update the key-material scope in
SECURITY.md to explicitly include ADMIN_NSEC alongside the mnemonic, identity
key, and derived trade keys, preserving the existing scope wording and coverage.
| - Flaws in the encryption, signing or validation of Nostr messages | ||
| (NIP-06, NIP-44, NIP-59, NIP-98). |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'SECURITY.md around vulnerability scope:\n'
sed -n '50,75p' SECURITY.md
printf '\nAll NIP mentions in SECURITY.md:\n'
rg -n 'NIP-[0-9]+' SECURITY.md || trueRepository: MostroP2P/mostro-cli
Length of output: 1420
🌐 Web query:
Nostr NIP-06 NIP-44 NIP-59 NIP-98 definition key derivation encryption signing validation HTTP authentication
💡 Result:
The requested Nostr Implementation Proposals (NIPs) define key management, encryption, privacy, and authentication standards for the protocol: NIP-06: Basic Key Derivation NIP-06 defines how to derive Nostr key pairs from mnemonic seed phrases [1][2]. It utilizes BIP-39 to generate the mnemonic seed and BIP-32 to derive keys using the path m/44'/1237'/0'/0/0, ensuring compatibility across different clients [1][3]. NIP-44: Versioned Encrypted Payloads NIP-44 provides a modern, versioned standard for end-to-end encryption, intended to replace the older NIP-04 [4]. Key features include: - Algorithms: Uses secp256k1 ECDH for key exchange, HKDF-SHA256 for key derivation, and ChaCha20-Poly1305 for authenticated encryption [5][4]. - Security: Adds message padding (to hide exact length) and utilizes nonces and version bytes to prevent security flaws found in earlier standards (e.g., chosen plaintext attacks) [4][6][7]. NIP-59: Gift Wrap Privacy Protocol NIP-59 defines a protocol to encapsulate events to protect metadata and provide sender privacy [8][9]. It uses three hierarchical concepts: - Rumor: An unsigned event containing the message content [8][10]. - Seal (Kind 13): A rumor encrypted (via NIP-44) and signed by the real sender's key [9][10]. - Gift Wrap (Kind 1059): The seal is encrypted again and wrapped in an event signed by a random, ephemeral key [8][10]. This hides the real author's identity from relays, which only see the ephemeral key and the recipient's information [9][10]. NIP-98: HTTP Authentication NIP-98 defines a method to authenticate HTTP requests using signed Nostr events [11][12]. - Mechanism: Clients create an ephemeral Kind 27235 event containing the target HTTP URL, the method (e.g., GET, POST), and an optional SHA-256 hash of the request body in a payload tag [12][13][14]. - Header: The client base64-encodes this signed event and sends it in the HTTP Authorization header as: Authorization: Nostr [11][14]. - Validation: Servers verify the signature, that the kind is 27235, and that the URL, method, and timestamp (typically within a 60-second window) match the request [12][13].
Citations:
- 1: https://github.com/nostr-protocol/nips/blob/master/06.md
- 2: https://nips.nostr.com/6
- 3: https://rust-nostr.org/sdk/nips/06.html
- 4: https://nostr.co.uk/nips/nip-44/
- 5: https://nips.nostr.com/44
- 6: NIP44 encryption standard, revision 3 nostr-protocol/nips#746
- 7: https://github.com/paulmillr/nip44
- 8: https://github.com/nostr-protocol/nips/blob/master/59.md
- 9: https://nostrcompass.org/en/topics/nip-59/
- 10: https://nostr-core.netlify.app/api/nip59
- 11: https://nip98.com/
- 12: https://github.com/nostr-protocol/nips/blob/master/98.md
- 13: https://nips.nostr.com/98
- 14: https://nostr-core.netlify.app/api/nip98
Correct the NIP classifications in the scope list.
NIP-06 covers mnemonic-based key derivation, and NIP-98 covers HTTP authentication with signed events; they do not belong under encryption, signing, or Nostr message validation. Keep NIP-44 and NIP-59 as encrypted or wrapped messaging, and list NIP-06 and NIP-98 separately.
🤖 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 `@SECURITY.md` around lines 63 - 64, Update the SECURITY.md scope list to
remove NIP-06 and NIP-98 from the encryption, signing, or Nostr message
validation entry, keeping NIP-44 and NIP-59 there. Add separate scope entries
identifying NIP-06 as mnemonic-based key derivation and NIP-98 as HTTP
authentication with signed events.
2 participants
Adds a SECURITY.md with the vulnerability reporting process for mostro-cli.
Summary by CodeRabbit