docs: add Stellar-specific security considerations to SECURITY.md (#812) - #825
docs: add Stellar-specific security considerations to SECURITY.md (#812)#825Ademiitura wants to merge 1 commit into
Conversation
|
@Ademiitura is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@Ademiitura Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughSECURITY.md adds Stellar-specific guidance covering network safety, private key handling, contract upgrades, and oracle manipulation risks and mitigations. ChangesStellar Security Guidance
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 81-84: Update the security guidance around contract admin/upgrade
keys and CI/CD automation keys to state that short-lived CI/CD signers must
never serve as the durable upgrade authority. Clarify that persistent authority
remains protected by the established multisig or governance process, and that
changing it requires an explicit, controlled migration and rotation.
- Around line 68-75: Update the network-passphrase checklist near the deployment
guidance to include Futurenet with the passphrase “Test SDF Future Network ;
October 2022,” alongside the existing Testnet and Mainnet values. Keep the CI
integration-test guidance consistent with the supported Futurenet network.
- Around line 98-100: Update the “Flash-loan-assisted manipulation” entry in the
security documentation to use Stellar ledger terminology instead of “single
transaction/block,” and qualify the risk as applicable only when the application
or surrounding protocol provides an atomic lending path; leave the other entries
unchanged.
- Line 84: Replace the `.gitignore`-focused guidance in SECURITY.md with active
secret-handling requirements: require protected secret storage, masked/no-echo
handling, and no logging of secret-bearing variables. Update the checklist to
explicitly enumerate SECRET_KEY, SERVER_PRIVATE_KEY, JWT_SECRET,
WEBHOOK_SIGNING_SECRET, ADMIN_API_KEY, and protected frontend VITE values
including VITE_CONTRACT_ID.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| Before deploying or interacting with contracts, confirm which network you're targeting: | ||
|
|
||
| - [ ] Verify `NETWORK_PASSPHRASE` matches the intended network (Testnet: `Test SDF Network ; September 2015`, Mainnet: `Public Global Stellar Network ; September 2015`). | ||
| - [ ] Confirm the Horizon/RPC endpoint URL points to the correct network before signing transactions. | ||
| - [ ] Never reuse Testnet keypairs for Mainnet accounts — treat them as fully separate identities. | ||
| - [ ] Double-check contract IDs; a valid contract ID on Testnet has no relationship to the same ID on Mainnet. | ||
| - [ ] Use a wallet or signer that clearly displays which network is active before every transaction approval. | ||
| - [ ] Run integration tests exclusively against Testnet/Futurenet; never point CI against Mainnet. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== FILES =="
git ls-files | rg '(^|/)SECURITY\.md$|readme|README' || true
echo "== SECURITY.md excerpt =="
if [ -f SECURITY.md ]; then
wc -l SECURITY.md
sed -n '55,85p' SECURITY.md | cat -n
fi
echo "== Network passphrase mentions =="
rg -n "Futurenet|Future Network|Testnet|Mainnet|NETWORK_PASSPHRASE|separator" SECURITY.md 2>/dev/null || true
echo "== Stellar passphrase documentation check =="
python3 - <<'PY'
# Read-only check of repository context only; no external request should block final if needed.
import subprocess, sys
print("python available:", sys.version.split()[0])
PYRepository: ritik4ever/stellar-stream
Length of output: 3328
🌐 Web query:
Stellar Futurenet network passphrase Test SDF Future Network October 2022
💡 Result:
The network passphrase for the Stellar Futurenet is "Test SDF Future Network; October 2022" [1][2][3]. This passphrase is a unique identifier used by the Stellar network to ensure transaction validity across different environments [1][4][5]. By including the network passphrase in the signature process, Stellar prevents transaction replay attacks, where a valid transaction on one network could otherwise be improperly broadcast or executed on another [5]. Developers typically use this passphrase when configuring their Stellar SDKs or tools to interact specifically with the Futurenet, which serves as a testing environment for bleeding-edge features [1][6]. If an incorrect passphrase is provided when signing a transaction, the transaction will be rejected by the network nodes [6][5].
Citations:
- 1: https://developers.stellar.org/docs/networks
- 2: https://developers.stellar.org/docs/networks/software-versions
- 3: https://stellar.github.io/js-stellar-base/network.js.html
- 4: https://developers.stellar.org/docs/learn/glossary
- 5: https://stellar.stackexchange.com/questions/1006/whats-networkpassphrase-used-for
- 6: https://developers.stellar.org/docs/tools/quickstart/network-modes
Include Futurenet in the network-passphrase checklist.
This section allows CI against Futurenet, but the passphrase list currently only covers Testnet and Mainnet. Add Test SDF Future Network ; October 2022, or explicitly scope the checklist to Testnet/Mainnet.
🤖 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 68 - 75, Update the network-passphrase checklist
near the deployment guidance to include Futurenet with the passphrase “Test SDF
Future Network ; October 2022,” alongside the existing Testnet and Mainnet
values. Keep the CI integration-test guidance consistent with the supported
Futurenet network.
| - Use multi-signature (multisig) setups for contract admin/upgrade keys rather than a single signer. | ||
| - Rotate deployment/admin keys if there's any suspicion of exposure, and prefer short-lived keys for CI/CD automation over long-lived ones. | ||
| - For frontend apps, never request or handle raw secret keys — rely on wallet-based signing (Freighter, xBull, etc.) so keys never touch application code. | ||
| - Environment variables holding sensitive keys should be scoped narrowly (per-environment) and excluded via `.gitignore`. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Separate persistent upgrade authority from CI signing keys.
Clarify that short-lived CI/CD signers must not be the contract’s durable admin/upgrade authority. The latter should remain behind the protected multisig/governance process described above, with rotation performed through an explicit controlled migration.
🤖 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 81 - 84, Update the security guidance around
contract admin/upgrade keys and CI/CD automation keys to state that short-lived
CI/CD signers must never serve as the durable upgrade authority. Clarify that
persistent authority remains protected by the established multisig or governance
process, and that changing it requires an explicit, controlled migration and
rotation.
| - Use multi-signature (multisig) setups for contract admin/upgrade keys rather than a single signer. | ||
| - Rotate deployment/admin keys if there's any suspicion of exposure, and prefer short-lived keys for CI/CD automation over long-lived ones. | ||
| - For frontend apps, never request or handle raw secret keys — rely on wallet-based signing (Freighter, xBull, etc.) so keys never touch application code. | ||
| - Environment variables holding sensitive keys should be scoped narrowly (per-environment) and excluded via `.gitignore`. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
rg -n --hidden -g '!.git' \
'(process\.env|import\.meta\.env|os\.environ|getenv|NETWORK_PASSPHRASE|SECRET|PRIVATE|SEED|KEY|TOKEN)'Repository: ritik4ever/stellar-stream
Length of output: 163
🏁 Script executed:
#!/bin/bash
set -eu
echo "## SECURITY.md around target line"
if [ -f SECURITY.md ]; then
wc -l SECURITY.md
sed -n '1,130p' SECURITY.md | nl -ba | sed -n '60,95p'
else
echo "SECURITY.md not found"
fi
echo
echo "## Relevant env/API/security references in repo"
rg -n \
-g '!/.git' \
'(process\.env|import\.meta\.env|os\.environ|getenv\(|NETWORK_PASSPHRASE|PASSPHRASE|SECRET|PRIVATE|SEED|KEY|TOKEN|API_KEY|API_SECRET|ENVS?|dotenv|dotenv-expand)' \
. || true
echo
echo "## README/config/env-like filenames"
fd -t f '^(README|SECURITY|env|.*env.*|.*config.*|.*secret.*|.*token.*|.*key.*)' . | sed -n '1,200p'Repository: ritik4ever/stellar-stream
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -eu
echo "## SECURITY.md lines 70-88"
sed -n '70,88p' SECURITY.md
echo
echo "## .env/.env.* / config-like secrets references, excluding generated frontend reports"
rg -n \
-g '!frontend/playwright-report' -g '!*.ts.snap' -g '!*.css' -g '!*.js' -g '!*.jsx' -g '!*.tsx' \
'\.env|VITE_.*SECRET|VITE_.*PRIVATE|VITE_.*TOKEN|VITE_.*KEY|VITE_.*SIGNING|JWT_SECRET|SERVER_PRIVATE_KEY|SECRET_KEY|WEBHOOK_SIGNING_SECRET|NETWORK_PASSPHRASE|CONTRACT_ID' \
. || true
echo
echo "## Read-only semantic probe: .gitignore controls untracked files only"
python3 - <<'PY'
from pathlib import Path
import subprocess
tmp = subprocess.check_output(["mktemp", "-d"], text=True).strip()
repo = Path(tmp) / "repo"
repo.mkdir()
(repo / ".git").mkdir()
(repo / ".gitignore").write_text(".env\n")
(repo / "tracked.txt").write_text("tracked")
(repo / ".env").write_text("SECRET=in_file")
(repo / "directory/.env").mkdir(parents=True)
(repo / "directory/.env").write_text("SECRET=in_directory")
patterns = [
("file .env untracked", str(repo / ".env")),
("directory .env untracked", str(repo / "directory/.env")),
("untracked sibling", str(repo / "secret.txt")),
]
for description, path in patterns:
out = subprocess.check_output(
["git", "-C", str(repo), "check-ignore", "-v", path],
stderr=subprocess.DEVNULL, text=True, check=False
)
print(f"{description}: {'ignored by gitignore' if out.strip() else 'not ignored by gitignore'}")
tracked = repo / "tracked.txt"
tracked.write_text("")
out = subprocess.check_output(
["git", "-C", str(repo), "check-ignore", "-v", str(tracked)],
stderr=subprocess.DEVNULL, text=True, check=False
)
print(f"tracked.txt: {'ignored by gitignore' if out.strip() else 'not ignored by gitignore'}")
PYRepository: ritik4ever/stellar-stream
Length of output: 40975
Replace .gitignore guidance with active secret handling requirements.
.gitignore only prevents untracked local files from being added; it does not protect secrets already injected into CI/deployment environments or prevent logging. Use protected secret storage and no-echo/masked handling for secret-bearing variables, and update the checklist to enumerate the repo’s secret env vars (SECRET_KEY, SERVER_PRIVATE_KEY, JWT_SECRET, WEBHOOK_SIGNING_SECRET, ADMIN_API_KEY, and protected frontend VITE values such as VITE_CONTRACT_ID).
🤖 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` at line 84, Replace the `.gitignore`-focused guidance in
SECURITY.md with active secret-handling requirements: require protected secret
storage, masked/no-echo handling, and no logging of secret-bearing variables.
Update the checklist to explicitly enumerate SECRET_KEY, SERVER_PRIVATE_KEY,
JWT_SECRET, WEBHOOK_SIGNING_SECRET, ADMIN_API_KEY, and protected frontend VITE
values including VITE_CONTRACT_ID.
| - **Price manipulation via low liquidity**: An attacker manipulates a thinly-traded pair to distort a price feed the contract relies on. | ||
| - **Flash-loan-assisted manipulation**: Large, short-term capital is used to skew an on-chain price within a single transaction/block. | ||
| - **Stale data reliance**: Contract logic uses outdated oracle data because it doesn't check the data's freshness/timestamp. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "SECURITY.md relevant section:"
sed -n '1,140p' SECURITY.md | nl -ba | sed -n '80,110p'
echo
echo "Search for Stellar ledger/block terminology:"
rg -n "block|ledger|flash" SECURITY.md || true
echo
echo "Repository files (top-level, relevant docs):"
git ls-files | sed -n '1,120p'Repository: ritik4ever/stellar-stream
Length of output: 233
🌐 Web query:
Stellar documentation ledger terminology difference block blockchain
💡 Result:
In the Stellar network, the term "ledger" serves a distinct purpose compared to the "block" terminology used in many other blockchain architectures [1][2]. A Stellar ledger represents the state of the entire network at a specific point in time [1][3]. It contains the complete set of all accounts, balances, smart contract data, and other persistent information at that moment [1][4][5]. In contrast, traditional blockchain "blocks" are typically viewed as individual units of data that store a collection of transactions, which are then linked to form a history [1]. While Stellar does not use "blocks" in the conventional sense to describe its primary state unit, it functions as a blockchain [1]. The network reaches consensus via the Stellar Consensus Protocol (SCP) to apply a set of transactions to the previous ledger, resulting in a new "last closed ledger" [1][3]. Each ledger is cryptographically linked to the previous one, creating an unbroken historical chain that functions analogously to a blockchain [1][6]. Key distinctions and technical nuances include: - State vs. Transactions: A Stellar ledger captures the current state of the entire network, whereas many other blockchains focus on the block as a container for transactions [1][7]. - Terminology Usage: While official documentation centers on "ledger," the term can sometimes contextually refer to either the individual state snapshot (the equivalent of a block) or the entire record of network history [1][2]. - History Archives: In infrastructure and archival contexts, Stellar documentation may still occasionally reference "blocks" when discussing the storage of compressed transaction logs in history archives, though "ledger" remains the standard term for the consensus unit [8]. In summary, Stellar uses ledgers as the atomic units of state consensus, which perform the same functional role as blocks in other blockchain systems [1][9].
Citations:
- 1: https://developers.stellar.org/docs/learn/fundamentals/stellar-data-structures/ledgers
- 2: https://docs.dune.com/data-catalog/stellar/evm_v_stellar
- 3: https://github.com/stellar/stellar-core/blob/master/src/ledger/readme.md
- 4: https://stellar.stackexchange.com/questions/73/terminology-whats-a-ledger-in-stellar
- 5: https://developers.stellar.org/docs/learn/fundamentals/stellar-data-structures
- 6: https://github.com/stellar/stellar-core/blob/master/docs/ledger.md
- 7: https://stellar.stackexchange.com/questions/1310/difference-between-stellar-ledger-and-blocks
- 8: https://github.com/stellar/stellar-core/blob/master/docs/architecture.md
- 9: https://deepwiki.com/stellar/stellar-core/1.2-key-concepts
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "SECURITY.md relevant section:"
sed -n '80,110p' SECURITY.md | awk '{printf "%4d %s\n", NR+79, $0}'
echo
echo "Security doc references to block/ledger/flash:"
grep -n -i 'block\|ledger\|flash' SECURITY.md || true
echo
echo "Repository files (top-level, relevant docs):"
git ls-files | sed -n '1,160p'Repository: ritik4ever/stellar-stream
Length of output: 9074
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "SECURITY.md top relevant section header/lines 86-95:"
sed -n '86,96p' SECURITY.md | awk '{printf "%4d %s\n", NR+85, $0}'Repository: ritik4ever/stellar-stream
Length of output: 978
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '95,100p' SECURITY.md | awk '{printf "%4d %s\n", NR+94, $0}'Repository: ritik4ever/stellar-stream
Length of output: 672
Use Stellar ledger terminology and qualify flash-loan attacks.
Line 99 still says single transaction/block; use ledger instead. Also note the flash-loan path is relevant only when the application or surrounding protocol exposes an atomic lending path, so the Stellar-specific section doesn’t imply a universal Stellar primitive.
🤖 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 98 - 100, Update the “Flash-loan-assisted
manipulation” entry in the security documentation to use Stellar ledger
terminology instead of “single transaction/block,” and qualify the risk as
applicable only when the application or surrounding protocol provides an atomic
lending path; leave the other entries unchanged.
|
Hi @Ademiitura, This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
2 participants
What changed
SECURITY.md, covering:Testing done
Related issues
Closes #812
Checklist
Summary by CodeRabbit