security: Add security enhancements, Horizon integration, and contract validations#746
Conversation
…lidations Changes across multiple components: CONTRACTS: Reject upgrade when new WASM hash matches current deployment - Prevents no-op re-upgrades that waste gas and confuse indexers - Added test for same-hash rejection behavior - Updated README documentation DASHBOARD: Replace mock useAccountOverview with real Horizon account fetch - Implemented horizon.ts with fetchAccountBalance and fetchAccountData - Updated useAccountOverview hook to use real Stellar Horizon API - Added comprehensive tests with mock fetch scenarios - Documented VITE_HORIZON_URL in .env.example SECURITY: Add CORS allowlist config for relayer - Implemented cors middleware with configurable ALLOWED_ORIGINS - Defaults to deny-all in production (empty ALLOWED_ORIGINS) - Added environment variable documentation - Enhanced .env.example with CORS configuration SECURITY.MD: Updated with detailed service boundaries and data flows - Added comprehensive service documentation (Relayer, Indexer, AI Agent) - Included data flow diagram and threat surface analysis - Cross-referenced THREAT_MODEL.md sections per service - Added CORS security and service-specific threat analysis Resolves ancore-org#562 ancore-org#563 ancore-org#661 ancore-org#663
|
@overprodigy 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! 🚀 |
📝 WalkthroughWalkthroughThis PR implements security hardening and data integration improvements across three areas: a CORS allowlist for the relayer service, real Horizon API account fetches for the dashboard (replacing mocks), same-hash upgrade rejection in the account contract, and corresponding security documentation updates covering threat modeling and service boundaries. ChangesSecurity, Relayer, Dashboard, and Contract Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
apps/web-dashboard/src/hooks/__tests__/useAccountOverview.test.ts (1)
78-109: ⚡ Quick winRefetch test doesn't prove a refetch occurred.
The mock returns the same payload before and after
refetch(), so asserting the balance is still50would pass even ifrefetchdid nothing. Change the mock between calls (e.g.mockResolvedValueOncewith a different balance) and assert the value updates to verify a fresh fetch.💚 Proposed change
- (global.fetch as any).mockResolvedValue( - new Response(JSON.stringify(mockAccountData), { - status: 200, - headers: { 'Content-Type': 'application/json' }, - }), - ); + (global.fetch as any) + .mockResolvedValueOnce( + new Response(JSON.stringify(mockAccountData), { status: 200 }), + ) + .mockResolvedValueOnce( + new Response( + JSON.stringify({ ...mockAccountData, balances: [{ balance: '75.0000000', asset_type: 'native' }] }), + { status: 200 }, + ), + ); @@ await waitFor(() => expect(result.current.isLoading).toBe(false)); expect(result.current.data?.balance).toBe(50); await result.current.refetch(); - expect(result.current.data?.balance).toBe(50); + await waitFor(() => expect(result.current.data?.balance).toBe(75));🤖 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 `@apps/web-dashboard/src/hooks/__tests__/useAccountOverview.test.ts` around lines 78 - 109, The test currently returns the same mock response before and after refetch so it can't verify a new network call; update the fetch mock in useAccountOverview.test by using mockResolvedValueOnce for the initial render (returning balance 50) and then mockResolvedValueOnce again with a different payload (e.g. balance 75) before calling result.current.refetch(), then assert result.current.data?.balance updates to the new value to prove refetch executed; reference the test around useAccountOverview, result.current.refetch, and the global.fetch mock to locate where to change the mocked responses.apps/web-dashboard/src/lib/horizon.ts (1)
34-74: ⚡ Quick winDuplicated request/error-handling and no timeout across both fetchers.
fetchAccountBalanceandfetchAccountDatarepeat identical URL construction and 404/non-OK handling, and neither guards against a hung request. Extract a shared helper and add a timeout viaAbortController.♻️ Proposed shared helper with timeout
+async function getAccount(publicKey: string, timeoutMs = 10000): Promise<HorizonAccountData> { + const url = `${HORIZON_URL}/accounts/${encodeURIComponent(publicKey)}`; + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), timeoutMs); + try { + const res = await fetch(url, { signal: controller.signal }); + if (!res.ok) { + if (res.status === 404) { + throw new Error(`Account not found: ${publicKey}`); + } + throw new Error(`Horizon error: ${res.status} ${res.statusText}`); + } + return (await res.json()) as HorizonAccountData; + } finally { + clearTimeout(timer); + } +} + export async function fetchAccountBalance(publicKey: string): Promise<number> { - const url = `${HORIZON_URL}/accounts/${publicKey}`; - - const res = await fetch(url); - if (!res.ok) { - if (res.status === 404) { - throw new Error(`Account not found: ${publicKey}`); - } - throw new Error(`Horizon error: ${res.status} ${res.statusText}`); - } - - const data = (await res.json()) as HorizonAccountData; + const data = await getAccount(publicKey); const nativeBalance = data.balances.find((b) => b.asset_type === 'native'); if (!nativeBalance) { throw new Error('No native balance found for account'); } return Number(nativeBalance.balance); } export async function fetchAccountData(publicKey: string): Promise<HorizonAccountData> { - const url = `${HORIZON_URL}/accounts/${publicKey}`; - - const res = await fetch(url); - if (!res.ok) { - if (res.status === 404) { - throw new Error(`Account not found: ${publicKey}`); - } - throw new Error(`Horizon error: ${res.status} ${res.statusText}`); - } - - return (await res.json()) as HorizonAccountData; + return getAccount(publicKey); }🤖 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 `@apps/web-dashboard/src/lib/horizon.ts` around lines 34 - 74, Extract the repeated fetch logic in fetchAccountBalance and fetchAccountData into a shared helper (e.g., fetchFromHorizon) that builds the URL, performs fetch with an AbortController timeout, awaits the response, and centralizes the 404/non-OK error handling (throwing the same errors currently used). Replace both fetchAccountBalance and fetchAccountData to call fetchFromHorizon(publicKey, { timeoutMs }) and then parse/return the JSON (and in fetchAccountBalance, keep the native balance lookup and throw the same "No native balance found for account" error if missing). Ensure the timeout aborts the request and the helper surfaces fetch/abort errors to callers.
🤖 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 @.env.example:
- Around line 19-21: Update the .env.example docs for ALLOWED_ORIGINS to remove
the "defaults to '*'" guidance and instead state that when ALLOWED_ORIGINS is
unset the service should deny all origins in production; provide a safe example
and note how to set a comma-separated allowlist for development (e.g.,
"http://localhost:3000,https://app.example.com"), and explicitly call out that
wildcard '*' is insecure for production. Ensure the comment references the
ALLOWED_ORIGINS variable so maintainers see the new deny-by-default behavior.
In `@apps/web-dashboard/src/hooks/useAccountOverview.ts`:
- Around line 39-40: The code is making redundant requests by calling
fetchAccountData(publicKey) and fetchAccountBalance(publicKey) against the same
/accounts/{publicKey} endpoint; instead, call fetchAccountData(publicKey) once,
extract the native balance from the returned accountData (e.g., find the
"native" asset in accountData.balances or accountData.balances.find(b =>
b.asset_type === 'native').balance), remove the fetchAccountBalance call and any
uses of its promise, and ensure downstream logic in useAccountOverview (where
balance is referenced) uses the derived balance from accountData.
- Line 44: The conversion nonce: Number(accountData.sequence) loses precision
for 64-bit Stellar sequence values; change AccountOverview.nonce to preserve the
raw sequence (use string or bigint) and stop coercing accountData.sequence to
Number in useAccountOverview (use accountData.sequence as-is), then update the
related prop types/components (NonceWidget, AccountOverview, and any
MetricWidget usages) to accept string | bigint (MetricWidget already accepts
string | number so adjust where necessary) so sequence values are passed through
without numeric precision loss.
In `@contracts/account/src/lib.rs`:
- Around line 410-413: The code calls
env.deployer().get_current_contract_wasm_hash() which is not in soroban-sdk;
instead persist the current wasm hash in contract storage (e.g., a storage key
like CURRENT_WASM_HASH) or accept it as an external parameter and read it via
storage APIs; replace the get_current_contract_wasm_hash() call with storage
read into current_wasm_hash (handle missing/zero-case), then compare
new_wasm_hash to that stored current_wasm_hash and return
Err(ContractError::InvalidWasmHash) if equal; update the related test
(test_upgrade_rejects_same_wasm_hash) to initialize or pass the stored wasm hash
accordingly so the comparison compiles and behaves as expected.
In `@SECURITY.md`:
- Around line 143-165: The fenced ASCII diagram block in SECURITY.md is missing
a language tag causing MD040 lint failures; update the opening code fence for
the diagram (the block containing the User Device / Ancore Backend / Soroban RPC
ASCII art) to include the language identifier `text` (i.e., change the leading
"```" to "```text") so the markdown linter recognizes it as a text block.
In `@services/relayer/src/server.ts`:
- Around line 72-77: The parseAllowedOrigins function currently returns '*' when
ALLOWED_ORIGINS is empty; change it to enforce a deny-all default in production
by checking NODE_ENV (e.g., if NODE_ENV === 'production' and envOrigins is falsy
return an empty array to deny all, otherwise allow the existing development
wildcard behaviour like returning '*'). Update the same pattern found in the
other CORS-related block referenced (the code around the second occurrence) so
both use NODE_ENV to decide between wildcard for dev and empty-array deny-all
for production, and ensure callers that expect a string[] handle the empty-array
case.
---
Nitpick comments:
In `@apps/web-dashboard/src/hooks/__tests__/useAccountOverview.test.ts`:
- Around line 78-109: The test currently returns the same mock response before
and after refetch so it can't verify a new network call; update the fetch mock
in useAccountOverview.test by using mockResolvedValueOnce for the initial render
(returning balance 50) and then mockResolvedValueOnce again with a different
payload (e.g. balance 75) before calling result.current.refetch(), then assert
result.current.data?.balance updates to the new value to prove refetch executed;
reference the test around useAccountOverview, result.current.refetch, and the
global.fetch mock to locate where to change the mocked responses.
In `@apps/web-dashboard/src/lib/horizon.ts`:
- Around line 34-74: Extract the repeated fetch logic in fetchAccountBalance and
fetchAccountData into a shared helper (e.g., fetchFromHorizon) that builds the
URL, performs fetch with an AbortController timeout, awaits the response, and
centralizes the 404/non-OK error handling (throwing the same errors currently
used). Replace both fetchAccountBalance and fetchAccountData to call
fetchFromHorizon(publicKey, { timeoutMs }) and then parse/return the JSON (and
in fetchAccountBalance, keep the native balance lookup and throw the same "No
native balance found for account" error if missing). Ensure the timeout aborts
the request and the helper surfaces fetch/abort errors to callers.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91c1f393-a11f-42fa-8b6e-33553e7df074
📒 Files selected for processing (10)
.env.exampleSECURITY.mdapps/web-dashboard/src/hooks/__tests__/useAccountOverview.test.tsapps/web-dashboard/src/hooks/useAccountOverview.tsapps/web-dashboard/src/lib/horizon.tscontracts/account/README.mdcontracts/account/src/lib.rsdocs/security/THREAT_MODEL.mdservices/relayer/package.jsonservices/relayer/src/server.ts
| # If not set, defaults to '*' (allow all origins) | ||
| # Example: "http://localhost:3000,https://app.example.com" | ||
| ALLOWED_ORIGINS= |
There was a problem hiding this comment.
Fix insecure CORS default guidance for production.
This docs block currently instructs an allow-all fallback when ALLOWED_ORIGINS is unset, which conflicts with the relayer security objective (empty should be deny-all in production). Please update the comment/example to reflect environment-specific safe defaults.
🤖 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 @.env.example around lines 19 - 21, Update the .env.example docs for
ALLOWED_ORIGINS to remove the "defaults to '*'" guidance and instead state that
when ALLOWED_ORIGINS is unset the service should deny all origins in production;
provide a safe example and note how to set a comma-separated allowlist for
development (e.g., "http://localhost:3000,https://app.example.com"), and
explicitly call out that wildcard '*' is insecure for production. Ensure the
comment references the ALLOWED_ORIGINS variable so maintainers see the new
deny-by-default behavior.
| const accountData = await fetchAccountData(publicKey); | ||
| const balance = await fetchAccountBalance(publicKey); |
There was a problem hiding this comment.
Redundant duplicate network request.
fetchAccountData and fetchAccountBalance both GET the same /accounts/{publicKey} endpoint, so this issues two identical, sequential requests (doubling latency and Horizon load). Fetch once and derive the native balance from accountData.
⚡ Proposed fix
- const accountData = await fetchAccountData(publicKey);
- const balance = await fetchAccountBalance(publicKey);
+ const accountData = await fetchAccountData(publicKey);
+ const balance = Number(
+ accountData.balances.find((b) => b.asset_type === 'native')?.balance ?? 0,
+ );📝 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.
| const accountData = await fetchAccountData(publicKey); | |
| const balance = await fetchAccountBalance(publicKey); | |
| const accountData = await fetchAccountData(publicKey); | |
| const balance = Number( | |
| accountData.balances.find((b) => b.asset_type === 'native')?.balance ?? 0, | |
| ); |
🤖 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 `@apps/web-dashboard/src/hooks/useAccountOverview.ts` around lines 39 - 40, The
code is making redundant requests by calling fetchAccountData(publicKey) and
fetchAccountBalance(publicKey) against the same /accounts/{publicKey} endpoint;
instead, call fetchAccountData(publicKey) once, extract the native balance from
the returned accountData (e.g., find the "native" asset in accountData.balances
or accountData.balances.find(b => b.asset_type === 'native').balance), remove
the fetchAccountBalance call and any uses of its promise, and ensure downstream
logic in useAccountOverview (where balance is referenced) uses the derived
balance from accountData.
| nonce: 42, | ||
| setData({ | ||
| balance, | ||
| nonce: Number(accountData.sequence), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the format and value range of a Stellar account sequence number? Is it an int64 and does the starting sequence equal ledger sequence shifted left by 32 bits?
💡 Result:
The Stellar account sequence number is a signed 64-bit integer (int64) [1]. Regarding the starting value, it is indeed derived from the ledger sequence number at the time of the account's creation. Specifically, an account's initial sequence number is calculated by taking the current ledger sequence number and shifting it left by 32 bits (ledger_sequence << 32) [2]. This mechanism was designed to ensure that account sequence numbers are sufficiently high, effectively separating them from older transaction sequence numbers and providing a large range for future increments [1]. Because the sequence number is a signed 64-bit integer, operations such as the Bump Sequence operation allow it to be modified, and care is taken to ensure it remains within valid bounds [1]. If a sequence number were to reach the maximum value for an int64, the account could no longer be used as a transaction source [1].
Citations:
- 1: https://github.com/stellar/stellar-protocol/blob/master/core/cap-0001.md
- 2: https://stellar.stackexchange.com/questions/2418/what-is-the-origin-of-an-accounts-initial-sequence-number
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/web-dashboard/src/hooks/useAccountOverview.ts"
echo "== file =="
stat "$file" || true
echo "== lines 1-120 =="
nl -ba "$file" | sed -n '1,120p'
echo "== find sequence/noncel usage near line 44 =="
nl -ba "$file" | sed -n '35,80p'
echo "== type of accountData and nonce =="
rg -n "accountData|sequence|nonce" "$file"Repository: ancore-org/ancore
Length of output: 194
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# locate file (avoid assuming path exists)
echo "== ls apps/web-dashboard/src/hooks =="
ls -R apps/web-dashboard/src/hooks 2>/dev/null || true
echo "== find matching filename =="
find . -path "*apps/web-dashboard/src/hooks*" -name "useAccountOverview.ts" 2>/dev/null || trueRepository: ancore-org/ancore
Length of output: 596
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/web-dashboard/src/hooks/useAccountOverview.ts"
echo "== head -n 120 =="
python3 - <<'PY'
import itertools,sys
path="apps/web-dashboard/src/hooks/useAccountOverview.ts"
with open(path,'r',encoding='utf-8') as f:
for i,line in zip(range(1,121),f):
print(f"{i:4d}: {line.rstrip()}")
PY
echo "== snippet around line 35-70 =="
python3 - <<'PY'
path="apps/web-dashboard/src/hooks/useAccountOverview.ts"
start,end=35,70
with open(path,'r',encoding='utf-8') as f:
for i,line in enumerate(f,1):
if start<=i<=end:
print(f"{i:4d}: {line.rstrip()}")
PY
echo "== rg for sequence definition / nonce type in the file =="
rg -n "sequence|nonce|interface|type|Number\(" "$file" || true
echo "== rg for accountData.sequence usage in other files =="
rg -n "accountData\.sequence" apps/web-dashboard/src || trueRepository: ancore-org/ancore
Length of output: 3419
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "function fetchAccountData|fetchAccountData" apps/web-dashboard/src/lib apps/web-dashboard/src || true
# show the implementation file(s) around fetchAccountData if found
files=$(rg -l "fetchAccountData" apps/web-dashboard/src | tr '\n' ' ')
echo "== files containing fetchAccountData =="
echo "$files"
for f in $files; do
echo "== $f =="
python3 - <<'PY'
import sys,re
path=sys.argv[1]
with open(path,'r',encoding='utf-8') as fh:
lines=fh.readlines()
for i,l in enumerate(lines,1):
if 'fetchAccountData' in l:
start=max(1,i-20); end=min(len(lines),i+60)
for j in range(start,end+1):
print(f"{j:4d}: {lines[j-1].rstrip()}")
break
PY
"$f" || true
doneRepository: ancore-org/ancore
Length of output: 875
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/web-dashboard/src/lib/horizon.ts"
echo "== show head -n 140 =="
python3 - <<'PY'
path="apps/web-dashboard/src/lib/horizon.ts"
with open(path,'r',encoding='utf-8') as f:
for i,line in zip(range(1,141),f):
print(f"{i:4d}: {line.rstrip()}")
PY
echo "== show around fetchAccountData (lines 50-110) =="
python3 - <<'PY'
path="apps/web-dashboard/src/lib/horizon.ts"
start,end=50,110
with open(path,'r',encoding='utf-8') as f:
for i,line in enumerate(f,1):
if start<=i<=end:
print(f"{i:4d}: {line.rstrip()}")
PYRepository: ancore-org/ancore
Length of output: 4001
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "\bnonce\b" apps/web-dashboard/src | head -n 200Repository: ancore-org/ancore
Length of output: 3914
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# inspect key files around nonce usage
files=(
apps/web-dashboard/src/widgets/auth-entry.ts
apps/web-dashboard/src/widgets/AccountWidgets.tsx
apps/web-dashboard/src/widgets/AccountOverviewGrid.tsx
apps/web-dashboard/src/hooks/useAccountOverview.test.ts
apps/web-dashboard/src/services/bulk-payouts.ts
apps/web-dashboard/src/pages/__tests__/ScheduledTransfers.test.tsx
)
for f in "${files[@]}"; do
echo "== $f =="
python3 - <<PY
path="$f"
try:
with open(path,'r',encoding='utf-8') as fh:
lines=fh.readlines()
except FileNotFoundError:
print("MISSING")
raise SystemExit(0)
for i,line in enumerate(lines,1):
if 1<=i<=160:
pass
# print whole file if <=200 lines else slice
if len(lines)<=220:
for i,l in enumerate(lines,1):
print(f"{i:4d}: {l.rstrip()}")
else:
for (start,end) in [(1,140),(140,220)]:
for i in range(start,min(end,len(lines))+1):
print(f"{i:4d}: {lines[i-1].rstrip()}")
PY
done
echo "== rg: from AccountOverview nonce into auth-entry or signing =="
rg -n "useAccountOverview\\(|AccountOverview|data\\.nonce|nonce:" apps/web-dashboard/src | head -n 200Repository: ancore-org/ancore
Length of output: 23102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "function MetricWidget|export const MetricWidget|MetricWidget" apps/web-dashboard/src/widgets -S
rg -n "AccountOverview.*nonce|data\\.nonce" apps/web-dashboard/src -S
# show MetricWidget implementation (if found)
f="$(rg -l "export const MetricWidget|function MetricWidget" apps/web-dashboard/src/widgets | head -n 1 || true)"
if [ -n "${f:-}" ]; then
echo "== MetricWidget file: $f =="
python3 - <<PY
path="$f"
with open(path,'r',encoding='utf-8') as fh:
lines=fh.readlines()
for i,l in enumerate(lines,1):
if i<=220:
print(f"{i:4d}: {l.rstrip()}")
PY
fi
echo "== useAccountOverview.test.ts =="
python3 - <<'PY'
path="apps/web-dashboard/src/hooks/__tests__/useAccountOverview.test.ts"
with open(path,'r',encoding='utf-8') as fh:
lines=fh.readlines()
for i,l in enumerate(lines,1):
if i<=180:
print(f"{i:4d}: {l.rstrip()}")
PYRepository: ancore-org/ancore
Length of output: 6875
Precision loss converting Stellar account sequence to nonce
In apps/web-dashboard/src/hooks/useAccountOverview.ts (nonce: Number(accountData.sequence)), HorizonAccountData.sequence is a string, but AccountOverview.nonce is a number. Stellar sequence numbers are signed int64 values that can exceed Number.MAX_SAFE_INTEGER, so this conversion can silently lose precision and produce an incorrect nonce. Keep nonce as the raw value (e.g., string and/or bigint) and update NonceWidget/AccountOverview/MetricWidget prop types accordingly (MetricWidget already supports string | number).
🤖 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 `@apps/web-dashboard/src/hooks/useAccountOverview.ts` at line 44, The
conversion nonce: Number(accountData.sequence) loses precision for 64-bit
Stellar sequence values; change AccountOverview.nonce to preserve the raw
sequence (use string or bigint) and stop coercing accountData.sequence to Number
in useAccountOverview (use accountData.sequence as-is), then update the related
prop types/components (NonceWidget, AccountOverview, and any MetricWidget
usages) to accept string | bigint (MetricWidget already accepts string | number
so adjust where necessary) so sequence values are passed through without numeric
precision loss.
| let current_wasm_hash = env.deployer().get_current_contract_wasm_hash(); | ||
| if new_wasm_hash == current_wasm_hash { | ||
| return Err(ContractError::InvalidWasmHash); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
soroban-sdk Deployer get_current_contract_wasm_hash method exists
💡 Result:
The method get_current_contract_wasm_hash does not exist in the soroban-sdk's Deployer struct [1]. According to the official documentation for the soroban-sdk, the Deployer struct provides functionality for deploying and managing smart contracts, including uploading Wasm code, deploying instances, and updating contract executables [2][1]. While it includes methods such as upload_contract_wasm and update_current_contract_wasm, there is no corresponding method to retrieve the hash of the currently executing contract's Wasm code directly from the Deployer [1].
Citations:
- 1: https://docs.rs/soroban-sdk/latest/soroban_sdk/deploy/struct.Deployer.html
- 2: https://docs.rs/soroban-sdk/latest/soroban_sdk/deploy/index.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for the method name in the repo
rg -n "get_current_contract_wasm_hash" .
# Locate the soroban-sdk version in Cargo.lock / Cargo.toml
if [ -f Cargo.lock ]; then
rg -n 'name = "soroban-sdk"|soroban-sdk' Cargo.lock | head -n 80
fi
rg -n "soroban-sdk" Cargo.toml **/Cargo.toml 2>/dev/null || true
# If there are vendored/patch overrides, list them
rg -n "\[patch\.crates-io\]|\[replace\]" -S Cargo.toml .cargo/config.toml **/Cargo.toml 2>/dev/null || trueRepository: ancore-org/ancore
Length of output: 336
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "get_current_contract_wasm_hash" contracts .cargo Cargo.toml Cargo.lock **/Cargo.toml 2>/dev/null || trueRepository: ancore-org/ancore
Length of output: 285
🌐 Web query:
soroban-sdk 21.7.0 Deployer get_current_contract_wasm_hash
💡 Result:
There is no method named get_current_contract_wasm_hash available on the Deployer struct or within the soroban-sdk version 21.7.0 [1][2][3]. The Deployer struct in the Soroban SDK provides functionality for contract deployment and management, such as: - upload_contract_wasm: Used to upload contract WASM code to the network, which returns the hash of the uploaded WASM [1]. - update_current_contract_wasm: Used to update the executable of the current contract, which requires a pre-uploaded WASM hash [1]. - with_current_contract: Used to create a deployer that deploys a contract with an ID derived from the currently executing contract's address and a provided salt [1][2]. - deploy_v2: Used to deploy a new contract instance using a previously uploaded WASM hash [2]. If you need to retrieve information about the currently executing contract or its WASM hash, you may need to look at the ledger or environment context directly, though there is no direct method corresponding to your query in the standard Deployer API [1][3].
Citations:
- 1: https://docs.rs/soroban-sdk/latest/soroban_sdk/deploy/struct.Deployer.html
- 2: https://docs.rs/soroban-sdk/latest/soroban_sdk/deploy/index.html
- 3: https://docs.rs/soroban-sdk/latest/soroban_sdk/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Confirm exact pin
echo "== contracts/Cargo.toml =="
cat -n contracts/Cargo.toml | sed -n '1,120p'
echo "== root Cargo.toml (if present) =="
ls -1 Cargo.toml >/dev/null 2>&1 && cat -n Cargo.toml | sed -n '1,120p' || true
echo "== Cargo.lock soroban-sdk entries (if present) =="
ls -1 Cargo.lock >/dev/null 2>&1 && rg -n 'name = "soroban-sdk"|soroban-sdk-' Cargo.lock | head -n 80 || true
# Inspect call sites
echo "== contracts/account/src/lib.rs: around 410 =="
sed -n '380,450p' contracts/account/src/lib.rs
echo "== contracts/account/src/lib.rs: around 1325 =="
sed -n '1280,1360p' contracts/account/src/lib.rs
# Look for any local trait/impl adding this method
echo "== local definitions mentioning get_current_contract_wasm_hash =="
rg -n "get_current_contract_wasm_hash" ./
echo "== anything that could be an extension trait for deployer =="
rg -n "trait .*Deployer|impl .*Deployer|DeployerExt|deployer\(\)\.get_" contracts/account/src/lib.rs contracts .cargo **/*.rs 2>/dev/null || trueRepository: ancore-org/ancore
Length of output: 5910
🌐 Web query:
site:docs.rs soroban-sdk 21.7.0 Deployer get_current_contract_wasm_hash
💡 Result:
There is no Deployer method named get_current_contract_wasm_hash in soroban-sdk 21.7.0 (or in the currently published Deployer API docs); instead, the Deployer API exposes methods like upload_contract_wasm, update_current_contract_wasm, and with_current_contract, which deal with uploading/replacing contract Wasm and deployment using a provided wasm hash.[1] What you can do with soroban-sdk deploy primitives: - Upload contract Wasm: Deployer::upload_contract_wasm(...) returns the wasm hash (BytesN<32>) that you later pass to deployment.[1] - Replace the current contract’s executable: Deployer::update_current_contract_wasm(wasm_hash) “Replaces the executable of the current contract with the provided Wasm” (the Wasm must already exist in the ledger via upload_contract_wasm).[1] If your goal is “get the currently running contract’s Wasm hash”, the official Deployer API shown in the soroban-sdk docs does not provide a getter for it; the supported pattern is to have the wasm hash from upload_contract_wasm (or manage it externally) and/or use Env/current-contract related helpers that provide addresses, not wasm hashes.[1][2]
Citations:
- 1: https://docs.rs/soroban-sdk/latest/soroban_sdk/deploy/struct.Deployer.html
- 2: https://docs.rs/soroban-sdk/latest/soroban_sdk/deploy/index.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -1 Cargo.lock 2>/dev/null || true
if [ -f Cargo.lock ]; then
rg -n 'name = "soroban-sdk"|soroban-sdk-' Cargo.lock | head -n 50
fi
rg -n '\[patch\.crates-io\]|\[replace\]' -S .cargo Cargo.toml **/Cargo.toml 2>/dev/null || trueRepository: ancore-org/ancore
Length of output: 43
🌐 Web query:
get_current_contract_wasm_hash soroban-sdk 21.7.0
💡 Result:
There is no function named get_current_contract_wasm_hash in soroban-sdk’s public docs; what soroban-sdk exposes for upgrading/changing the executing contract’s code is env.deployer.update_current_contract_wasm(wasm_hash) (and related deploy/upload helpers), which takes the new Wasm hash you already uploaded to the ledger. [1][2][3] If what you want is the current contract’s executable Wasm hash from inside a contract, the sources found only describe getting the current contract address via Env::current_contract_address, not a “current contract wasm hash” accessor. [4] What soroban-sdk 21.7.0 likely has vs “current” APIs: the docs we found are for latest and for Deployer/update_current_contract_wasm; we did not find a 21.7.0-specific definition for get_current_contract_wasm_hash in the evidence gathered, so I can’t confirm its existence or signature for 21.7.0 from the cited sources. [5][1][2] Practical alternative grounded in docs: to upgrade the currently executing contract, define an admin-authorized upgrade that calls env.deployer.update_current_contract_wasm(new_wasm_hash); the new_wasm_hash is the Sha256 hash of the uploaded executable (which must already be present on-chain). [3]
Citations:
- 1: https://docs.rs/soroban-sdk/latest/soroban_sdk/deploy/struct.Deployer.html
- 2: https://docs.rs/soroban-sdk/latest/soroban_sdk/deploy/index.html
- 3: https://developers.stellar.org/docs/build/guides/conventions/upgrading-contracts
- 4: https://docs.rs/soroban-sdk/latest/soroban_sdk/struct.Env.html
- 5: https://docs.rs/crate/soroban-sdk/21.7.7
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# show crate deps resolved
ls -1 Cargo.lock 2>/dev/null && rg -n 'name = "soroban-sdk"|soroban-sdk-' Cargo.lock | head -n 80 || true
# inspect top of contracts/account/src/lib.rs for imports/traits
sed -n '1,120p' contracts/account/src/lib.rs
# inspect around upgrade for context (imports in scope, types)
sed -n '360,440p' contracts/account/src/lib.rs
# inspect autour of test using get_current_contract_wasm_hash
sed -n '1290,1345p' contracts/account/src/lib.rs
# verify no local trait/impl defines it
rg -n "trait .*Deployer|impl .*Deployer|Deployer.*get_|get_current_contract_wasm_hash" contracts/account/src/lib.rs contracts .cargo 2>/dev/null || trueRepository: ancore-org/ancore
Length of output: 8198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Has Cargo.lock?"
ls -ლა Cargo.lock 2>/dev/null || true
echo "Search soroban-sdk declarations/overrides:"
rg -n "soroban-sdk" -S . --glob '!target/**'
echo "Search for any trait/impl/extension that defines get_current_contract_wasm_hash:"
rg -n "get_current_contract_wasm_hash" -S . --glob '*.rs'
echo "Check for overrides in .cargo config:"
ls -la .cargo 2>/dev/null || true
rg -n "\[patch\.crates-io\]|\[replace\]" -S .cargo Cargo.toml **/Cargo.toml 2>/dev/null || trueRepository: ancore-org/ancore
Length of output: 1712
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== contracts/Cargo.lock soroban-sdk version =="
rg -n "name = \"soroban-sdk\"|version = \"[0-9]+\.[0-9]+\.[0-9]+\"" contracts/Cargo.lock | head -n 80
# Extract the exact soroban-sdk package block around its entry
python3 - <<'PY'
import re, itertools, pathlib
p = pathlib.Path("contracts/Cargo.lock")
txt = p.read_text()
# find block for soroban-sdk
m = re.search(r'\[\[package\]\]\nname = "soroban-sdk"\nversion = "([^"]+)"', txt)
print("resolved soroban-sdk version:", m.group(1) if m else "NOT FOUND")
PY
echo "== contracts/account/Cargo.toml =="
cat -n contracts/account/Cargo.toml | sed -n '1,120p'
echo "== contracts/validation-modules/Cargo.toml =="
cat -n contracts/validation-modules/Cargo.toml | sed -n '1,120p'
echo "== contracts/Cargo.toml =="
cat -n contracts/Cargo.toml | sed -n '1,120p'
echo "== .cargo/config.toml (if any) =="
if [ -f .cargo/config.toml ]; then cat -n .cargo/config.toml; fi
if [ -f .cargo/config ]; then cat -n .cargo/config; fiRepository: ancore-org/ancore
Length of output: 4110
🌐 Web query:
site:docs.rs soroban-sdk 21.7.7 Deployer get_current_contract_wasm_hash
💡 Result:
There is no function named get_current_contract_wasm_hash in the soroban-sdk [1][2][3]. In the Soroban SDK, while you can interact with Wasm hashes using the Deployer (e.g., uploading Wasm via upload_contract_wasm or updating it via update_current_contract_wasm), there is no direct, public SDK method in the Deployer or elsewhere to retrieve the Wasm hash of the currently executing contract [1][2]. If you need to manage or track the Wasm hash of your contract, a common pattern is to store the hash in persistent storage during the contract's initialization or update process [4]. You can then retrieve this value from storage using the standard env.storage API [4].
Citations:
- 1: https://docs.rs/soroban-sdk/latest/soroban_sdk/deploy/struct.Deployer.html
- 2: https://docs.rs/soroban-sdk/latest/soroban_sdk/deploy/index.html
- 3: https://docs.rs/soroban-sdk/latest/soroban_sdk/
- 4: https://docs.rs/soroban-sdk/latest/soroban_sdk/attr.contracttype.html
🌐 Web query:
get_current_contract_wasm_hash soroban-sdk
💡 Result:
There is no direct method named get_current_contract_wasm_hash in the current soroban-sdk [1][2][3]. The Soroban environment does not expose the WASM hash of the currently executing contract directly to the contract itself [1][2]. If you need to identify or work with contract WASM hashes in Soroban, consider the following approaches: 1. Contract Identification: The environment provides methods to get the address of the current contract via env.current_contract_address [1]. 2. Deployment and WASM Management: When deploying or upgrading contracts, you typically manage the WASM hash explicitly [4]. For example, when using env.deployer.upload_contract_wasm(wasm_bytes), the SDK returns the WASM hash, which is then used for deployment [4]. 3. Ledger Data: If you need to retrieve the WASM hash of a contract that is already deployed on the network, you must query the network's ledger state (e.g., via Horizon or RPC) to inspect the ContractCodeEntry associated with that contract's address [5]. While there have been discussions regarding executable updates and WASM management in the Soroban environment, the SDK remains focused on deploying and interacting with contracts using known WASM hashes rather than introspecting them at runtime [6][4].
Citations:
- 1: https://docs.rs/soroban-sdk/latest/soroban_sdk/struct.Env.html
- 2: https://docs.rs/soroban-sdk/latest/soroban_sdk/
- 3: https://docs.rs/soroban-sdk
- 4: https://docs.rs/soroban-sdk/latest/soroban_sdk/deploy/index.html
- 5: https://developers.stellar.org/docs/build/guides/transactions/upload-wasm-bytecode
- 6: Calling
update_current_contract_wasmfrom contract in test doesn't work stellar/rs-soroban-env#1089
Fix unsupported Deployer::get_current_contract_wasm_hash usage in same-hash rejection
- Same-hash rejection placement is correct (after the all-zero guard, before version bump/state updates).
env.deployer().get_current_contract_wasm_hash()is not part of the soroban-sdk Deployer API (repo resolves tosoroban-sdk 21.7.7), so this code (andtest_upgrade_rejects_same_wasm_hashat ~1325) will not compile—store the current wasm hash in contract storage (or pass/query it externally) and compare againstnew_wasm_hashinstead.
🤖 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 `@contracts/account/src/lib.rs` around lines 410 - 413, The code calls
env.deployer().get_current_contract_wasm_hash() which is not in soroban-sdk;
instead persist the current wasm hash in contract storage (e.g., a storage key
like CURRENT_WASM_HASH) or accept it as an external parameter and read it via
storage APIs; replace the get_current_contract_wasm_hash() call with storage
read into current_wasm_hash (handle missing/zero-case), then compare
new_wasm_hash to that stored current_wasm_hash and return
Err(ContractError::InvalidWasmHash) if equal; update the related test
(test_upgrade_rejects_same_wasm_hash) to initialize or pass the stored wasm hash
accordingly so the comparison compiles and behaves as expected.
| ``` | ||
| ┌─────────────────────────────────────────────────────────────────┐ | ||
| │ User Device (Trusted, partially) │ | ||
| │ ┌──────────────────────┐ ┌──────────────────────┐ │ | ||
| │ │ Extension Wallet │◄────►│ Local Encrypted │ │ | ||
| │ │ (sign, auth, keys) │ │ Storage (PBKDF2+GCM) │ │ | ||
| │ └──────────────────────┘ └──────────────────────┘ │ | ||
| └────┬────────────────────────────────────────────────────────────┘ | ||
| │ HTTPS | ||
| ▼ | ||
| ┌─────────────────────────────────────────────────────────────────┐ | ||
| │ Ancore Backend (CORS restricted) │ | ||
| │ ┌──────────────┐ ┌──────────────┐ ┌──────────────┐ │ | ||
| │ │ Relayer │ │ Indexer │ │ AI Agent │ │ | ||
| │ │ (relay tx) │ │ (read state) │ │ (validate) │ │ | ||
| │ └──────────────┘ └──────────────┘ └──────────────┘ │ | ||
| └────┬──────────────────┬──────────────────────────────────────────┘ | ||
| │ HTTPS │ HTTPS | ||
| ▼ ▼ | ||
| Soroban RPC Stellar Horizon | ||
| (invoke, (account, balance, | ||
| submit) history) | ||
|
|
There was a problem hiding this comment.
Add a language tag to the fenced diagram block.
The code fence is missing a language identifier (MD040). Use text to keep markdown lint clean.
Suggested fix
-```
+```text
┌─────────────────────────────────────────────────────────────────┐
...
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 143-143: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 143 - 165, The fenced ASCII diagram block in
SECURITY.md is missing a language tag causing MD040 lint failures; update the
opening code fence for the diagram (the block containing the User Device /
Ancore Backend / Soroban RPC ASCII art) to include the language identifier
`text` (i.e., change the leading "```" to "```text") so the markdown linter
recognizes it as a text block.
| function parseAllowedOrigins(): string[] | string { | ||
| const envOrigins = process.env.ALLOWED_ORIGINS; | ||
| if (!envOrigins) { | ||
| return '*'; // Default to wildcard for development | ||
| } | ||
| return envOrigins.split(',').map((origin) => origin.trim()); |
There was a problem hiding this comment.
Enforce deny-all default in production when ALLOWED_ORIGINS is empty.
Current logic falls back to '*' unconditionally, which violates the required production-safe default and weakens CORS hardening.
Suggested fix
function parseAllowedOrigins(): string[] | string {
const envOrigins = process.env.ALLOWED_ORIGINS;
- if (!envOrigins) {
- return '*'; // Default to wildcard for development
- }
- return envOrigins.split(',').map((origin) => origin.trim());
+ const isProduction = process.env.NODE_ENV === 'production';
+ if (!envOrigins || envOrigins.trim() === '') {
+ return isProduction ? [] : '*';
+ }
+ return envOrigins
+ .split(',')
+ .map((origin) => origin.trim())
+ .filter(Boolean);
}Also applies to: 89-97
🤖 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 `@services/relayer/src/server.ts` around lines 72 - 77, The parseAllowedOrigins
function currently returns '*' when ALLOWED_ORIGINS is empty; change it to
enforce a deny-all default in production by checking NODE_ENV (e.g., if NODE_ENV
=== 'production' and envOrigins is falsy return an empty array to deny all,
otherwise allow the existing development wildcard behaviour like returning '*').
Update the same pattern found in the other CORS-related block referenced (the
code around the second occurrence) so both use NODE_ENV to decide between
wildcard for dev and empty-array deny-all for production, and ensure callers
that expect a string[] handle the empty-array case.
2 participants
What
Comprehensive security, frontend, and contract improvements across multiple components.
Issues Resolved
Closes #562
Closes #563
Closes #661
Closes #663
Changes
#562 - CONTRACTS: Reject upgrade when new WASM hash matches current deployment
#563 - DASHBOARD: Replace mock useAccountOverview with real Horizon account fetch
#663 - SECURITY: Add CORS allowlist config for relayer
#661 - SECURITY.MD: Updated with detailed service boundaries and data flows
Summary by CodeRabbit
New Features
Documentation
Tests