Skip to content

fix(contract): add access control to set_risk_tier — closes #50#74

Merged
mericcintosun merged 2 commits into
mericcintosun:mainfrom
jobbykings:fix/issue-50-set-risk-tier-access-control
Jul 14, 2026
Merged

fix(contract): add access control to set_risk_tier — closes #50#74
mericcintosun merged 2 commits into
mericcintosun:mainfrom
jobbykings:fix/issue-50-set-risk-tier-access-control

Conversation

@jobbykings

Copy link
Copy Markdown
Contributor

Summary

Fixes #50[Security] set_risk_tier has no access control — any address can overwrite any user's score

Without authorization checks, any on-chain actor could call set_risk_tier to manipulate any user's credit score, making the entire system untrustworthy and blocking mainnet deployment.

Changes

risk_score/src/lib.rs

What Why
initialize(admin) Sets a trusted admin address once; panics on re-initialization
get_admin() Transparency helper to read the stored admin
set_risk_tier — auth guard Requires caller to be admin OR the user themselves via require_auth()
update_chosen_tier — auth guard Requires the user themselves via require_auth()
setup() test helper Calls initialize + mock_all_auths so all existing tests keep passing

New tests (7)

  • test_initialize_sets_admin
  • test_initialize_twice_panics
  • test_admin_can_set_risk_tier_for_any_user
  • test_user_can_set_own_risk_tier
  • test_third_party_cannot_set_risk_tier_for_another_user ← key security test
  • test_user_can_update_own_chosen_tier
  • test_third_party_cannot_update_chosen_tier ← key security test

All 18 pre-existing tests were updated to use the setup() helper and continue to pass.

Testing

cd risk_score
cargo test

Note: Rust/Cargo is not available in the CI environment of this fork. The contract compiles and all tests pass locally with soroban-sdk 22.

…tier

Fixes mericcintosun#50

- Add initialize(admin) function that sets a trusted admin address once
- set_risk_tier now requires caller to be admin OR the user themselves
- update_chosen_tier now requires the user themselves (require_auth)
- Add get_admin() helper for transparency
- Add 7 new auth tests covering all access paths:
  - test_initialize_sets_admin
  - test_initialize_twice_panics
  - test_admin_can_set_risk_tier_for_any_user
  - test_user_can_set_own_risk_tier
  - test_third_party_cannot_set_risk_tier_for_another_user
  - test_user_can_update_own_chosen_tier
  - test_third_party_cannot_update_chosen_tier
- Refactor all existing tests to use setup() helper that calls initialize()

Signed-off-by: Daniel Job Gonsum <danieljob003@gmail.com>
@vercel

vercel Bot commented Apr 29, 2026

Copy link
Copy Markdown

@jobbykings is attempting to deploy a commit to the mericcintosun Team on Vercel.

A member of the Team first needs to authorize it.

…tier

Fixes mericcintosun#50

Problem: set_risk_tier had no authorization check — any address could
overwrite any user's risk score, making the credit system trivially
manipulable and blocking mainnet deployment.

Solution:
- Add initialize(admin) — sets a trusted admin address once; panics on
  re-initialization
- Split set_risk_tier into two entry points:
  - set_risk_tier: user.require_auth() — user signs for themselves
  - admin_set_risk_tier: admin.require_auth() — oracle/backend flow
- update_chosen_tier: user.require_auth() — only the user themselves
- Add get_admin() transparency helper
- Extract write_risk_tier() private helper to avoid duplication
- Add 5 new auth tests:
  - test_initialize_sets_admin
  - test_initialize_twice_panics
  - test_user_can_set_own_risk_tier
  - test_third_party_cannot_set_risk_tier_for_another_user
  - test_admin_can_set_risk_tier_for_any_user
  - test_non_admin_cannot_call_admin_set_risk_tier
  - test_user_can_update_own_chosen_tier
  - test_third_party_cannot_update_chosen_tier
- Refactor all existing tests to use setup() helper (initialize first)

Note: env.invoker() does not exist in the Soroban SDK. The correct
pattern is address.require_auth() on the specific principal that must
sign. The 'admin OR user' pattern is implemented as two separate
functions with distinct auth requirements.

Signed-off-by: Daniel Job Gonsum <danieljob003@gmail.com>
@mericcintosun mericcintosun merged commit 249a4ca into mericcintosun:main Jul 14, 2026
1 check failed
mericcintosun added a commit that referenced this pull request Jul 15, 2026
…isk-score bug

- All 6 active gaps re-verified against current code (still open)
- New: risk model returned (1 - p) * 100 while the weights already yield
  P(risky), so risky wallets received LOW scores -> found, fixed, moved to
  resolved
- Flag the model recalibration as a behaviour change needing product review
- Record resolved: contract auth (#74), CSP (verified live), Vercel build,
  repo artifact hygiene
- Note API contract bug: invalid sort returns 500 instead of 400

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mericcintosun added a commit that referenced this pull request Jul 15, 2026
… work

Closes the two gaps that made the risk score meaningless: the browser both
computed AND submitted its own score, and the write path never worked.

Server-side risk oracle
- New POST /api/risk/attest + src/lib/server/riskOracle.js: the score is
  re-derived from Horizon data the SERVER fetches (src/lib/server/horizonMetrics.js,
  mirroring the client formulas) and written with admin_set_risk_tier, signed by
  the contract admin key. A user can no longer self-report a score.
- The transaction goes through prepareTransaction() (simulation), which attaches
  the footprint, resource fees and the authorization entries that
  admin.require_auth() needs — the old client path skipped simulation entirely
  and silently fell back to writing account manageData instead of the contract.
- Verified against testnet: tx 3f983a678bf191d0ed7b790d507b7f14a80e55314ff18cd7382e2d0d4caefb00
  -> on-chain {"score":57,"tier":"TIER_2"}, can_access_tier(TIER_2) = true.

Rate limiting moved server-side
- The 24h cooldown is now enforced from the contract's own timestamp: stateless,
  and no longer bypassable by clearing localStorage. Verified: second call -> 429.

Contract actually deployed with auth
- PR #74 added require_auth to the source, but the chain still ran the old
  unauthenticated contract — and both recorded contract ids no longer existed on
  testnet at all (reset), so every read/write was failing.
- Deployed CCGZV37C3FC2GLVNIHFEC6OVDHRFLQCELPTQLII44Z7RXZBEER5POPRO and ran
  initialize(admin). Auth verified on-chain: an unauthorized set_risk_tier is
  rejected. Replaced the dead contract id across 7 files (env, CI, docs).

Removed the self-service write path
- src/app/lib/writeScore.js deleted: every export had zero callers once both
  call sites moved to the oracle. page.js previously wrote a score derived from
  values the USER typed into a form. This also removes one "kalepail" public-seed
  usage. src/lib/writeScore.js (dead set_score caller) deleted too.

Bug fix
- AutomatedRiskAnalyzer never awaited the async checkRateLimit and stored the
  Promise in state, so rateLimitStatus.canUpdate was always undefined and the
  "Update Score" button rendered permanently disabled.

Tests 170/170, lint and build green. gaps.md updated: 6 active gaps -> 3.

Co-Authored-By: Claude Opus 4.8 (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.

[Security] set_risk_tier has no access control — any address can overwrite any user's score

2 participants