Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
44 changes: 44 additions & 0 deletions .github/SECURITY_REVIEW_CHECKLIST.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Security Review Checklist — Secrets, PII & Transport

**Use for:** any PR that adds/changes a data store, secret, credential, external integration, or network-facing endpoint.
**Companion doc:** [`docs/security/encryption-review.md`](../docs/security/encryption-review.md) — full current-state review this checklist enforces going forward.
**Not this checklist:** smart-contract vulnerability classes (reentrancy, access control, gas/DoS) are covered by [`docs/SECURITY_CHECKLIST.md`](../docs/SECURITY_CHECKLIST.md).

---

## 1. Secrets & Credentials

- [ ] No secret, API key, private key, or credential is hardcoded or committed (checked by `gitleaks` in CI, but review the diff yourself too)
- [ ] New secrets are read from `process.env` / GitHub Actions `secrets.*`, never from a config file checked into the repo
- [ ] Any new required secret is documented in [`docs/ENV_VARIABLE_MATRIX.md`](../docs/ENV_VARIABLE_MATRIX.md)
- [ ] Secrets that must exist in production fail startup loudly if missing or weak (follow the pattern in `backend/src/auth.ts`'s `assertJwtSecretValid()`) rather than silently falling back to a dev default
- [ ] Tokens/keys are persisted as one-way hashes (SHA-256/HMAC or stronger), never in plaintext

## 2. Data at Rest

- [ ] New sensitive fields (PII, tokens, wallet-linked identifiers) are added to the correct model with the retention category noted in [`docs/DATA_RETENTION_DELETION_POLICY.md`](../docs/DATA_RETENTION_DELETION_POLICY.md)
- [ ] Any new database, cache, or file-based store this PR introduces is confirmed to write to the *intended* backing store — verify the Prisma datasource / connection string actually points where you think it does (see `docs/security/encryption-review.md` §4 F1 for a real example of this silently going wrong)
- [ ] No secret or PII value is written to logs, error messages, or audit trails in plaintext — check against `backend/src/auditRedaction.ts`'s redaction patterns

## 3. Data in Transit

- [ ] All new outbound calls (webhooks, RPC, third-party APIs) use `https://` / `wss://`, never plaintext `http://`
- [ ] New database or cache connection strings enforce TLS (`sslmode=require` for Postgres, `rediss://` for Redis) where the backing service supports it
- [ ] New cookies (if any) set `Secure`, `HttpOnly`, and an explicit `SameSite` — this repo's auth model currently uses Bearer tokens, not cookies, so introducing a cookie is a deliberate change worth flagging in the PR description
- [ ] CORS changes stay within an explicit origin allowlist — no `*` or broad regex in `CORS_ALLOWED_ORIGINS`

## 4. PII & Sensitive Data Handling

- [ ] User-identifying data (wallet address, email, IP) is only collected/stored where there's a documented purpose in [`docs/DATA_RETENTION_DELETION_POLICY.md`](../docs/DATA_RETENTION_DELETION_POLICY.md)
- [ ] Frontend `localStorage`/`sessionStorage` usage stores no private keys, JWTs, or API keys — only public/non-sensitive values (wallet address, UI prefs, session timestamps)
- [ ] New admin or export endpoints that expose PII/financial data are gated by the correct RBAC role and rate-limited

## 5. If This PR Touches Any of the Findings in `docs/security/encryption-review.md`

- [ ] Confirm which finding (F1–F6) this PR addresses, if any
- [ ] Update the finding's status in `docs/security/encryption-review.md` in the same PR
- [ ] If this PR resolves a finding, note it in the PR description so reviewers can verify against the documented evidence

---

**If any box above cannot be checked**, explain why in the PR description rather than leaving it silently unchecked — a documented exception is reviewable; a missing explanation is not.
5 changes: 5 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Repository Guidelines

## Git & PR Rules
- NEVER add co-author tags, AI signatures, or attribution footers to git commit messages or PR descriptions.
- Keep commits concise and strictly attributed to the git user identity.
2 changes: 1 addition & 1 deletion backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"dev": "tsx watch src/index.ts",
"build": "tsc",
"start": "node dist/index.js",
"test": "jest",
"test": "jest --runInBand",
"test:smoke": "npm run build && npm run start &",
"lint": "eslint src",
"format": "prettier --write src",
Expand Down
Binary file modified backend/prisma/dev.db
Binary file not shown.
5 changes: 3 additions & 2 deletions contracts/share-price-math/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Vault share conversion math with deterministic rounding policy.
//!
//! Host-buildable library used by the Soroban vault contract, proptest suite,
//! and `cargo fuzz` targets.
//! `no_std` so it links cleanly into the Soroban vault's `wasm32-unknown-unknown`
//! build; also used host-side by the proptest suite and `cargo fuzz` targets.
//!
//! ## Numeric Boundaries and Constraints
//! - Maximum `i128` (3.4e38) represents the theoretical absolute upper limit for assets or shares.
Expand All @@ -11,6 +11,7 @@
//! should generally be kept significantly below `i128::MAX / expected_deposit_size`.
//! - Specifically, if `total_shares` and `total_assets` are bounded by `2^64`, then any operation with
//! values up to `2^63` will never overflow an `i128`.
#![no_std]

pub mod fuzz_invariants;
pub mod rounding;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Seeds for failure cases proptest has generated in the past. It is
# automatically read and these particular cases re-run before any
# novel cases are generated.
#
# It is recommended to check this file in to source control so that
# everyone who runs the test benefits from these saved cases.
cc 9500b952ba76ddc8412c8075347f03164d293671f7119543f0c58b98caa97ad2 # shrinks to deposit_amount = 1000, cooldown_secs = 1
18 changes: 12 additions & 6 deletions contracts/vault/src/admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,18 @@ mod tests {
#[test]
fn test_admin_proposal_nonce_is_monotonic() {
let env = Env::default();
assert_eq!(next_proposal_id(&env), 1);
assert_eq!(next_proposal_id(&env), 2);
assert_eq!(next_proposal_id(&env), 3);
let contract_id = env.register(crate::YieldVault, ());
env.as_contract(&contract_id, || {
assert_eq!(next_proposal_id(&env), 1);
assert_eq!(next_proposal_id(&env), 2);
assert_eq!(next_proposal_id(&env), 3);
});
}

#[test]
fn test_admin_proposal_round_trip() {
let env = Env::default();
let contract_id = env.register(crate::YieldVault, ());
let proposer = Address::generate(&env);
let new_admin = Address::generate(&env);
let proposal = AdminProposal {
Expand All @@ -62,8 +66,10 @@ mod tests {
cancelled: false,
created_at: 42,
};
write_proposal(&env, 1, &proposal);
let loaded = read_proposal(&env, 1).expect("proposal stored");
assert_eq!(loaded, proposal);
env.as_contract(&contract_id, || {
write_proposal(&env, 1, &proposal);
let loaded = read_proposal(&env, 1).expect("proposal stored");
assert_eq!(loaded, proposal);
});
}
}
8 changes: 7 additions & 1 deletion contracts/vault/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,13 @@ pub enum VaultError {
ProposalRejected = 36,

// ── Oracle / treasury / strategy health (27–29, 37) ──────────────────────
/// Oracle validation failed (stale or manipulated price).
/// Oracle validation failed: expired heartbeat, invalid price data (zero,
/// negative, future timestamp, unsafe decimals/overflow), excessive
/// deviation from the last validated price, or an out-of-range
/// `set_oracle_heartbeat` configuration. See [`crate::oracle`] for the
/// full stale-data policy. Reused across all of `oracle::OracleError`'s
/// variants rather than spending a dedicated code per variant under the
/// 50-case cap.
OracleValidationFailed = 27,
/// Treasury claim quota exceeded for the current epoch.
ClaimQuotaExceeded = 28,
Expand Down
8 changes: 1 addition & 7 deletions contracts/vault/src/feature_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ fn test_role_restricted_pausability_controls() {
vault.initialize(&admin, &usdc);

let pauser = Address::generate(&env);
let unauthorized = Address::generate(&env);
let _unauthorized = Address::generate(&env);

// Initial pauser is None
assert_eq!(vault.pauser(), None);
Expand All @@ -337,9 +337,6 @@ fn test_role_restricted_pausability_controls() {

// Designated pauser can pause with role
vault.pause_with_role(&pauser, &PauseReason::SecurityIncident);
vault
.pause_with_role(&pauser, &PauseReason::SecurityIncident)
.unwrap();
assert!(vault.is_paused());
assert_eq!(vault.pause_reason(), Some(PauseReason::SecurityIncident));

Expand All @@ -350,9 +347,6 @@ fn test_role_restricted_pausability_controls() {

// Admin can also pause and unpause with role
vault.pause_with_role(&admin, &PauseReason::Maintenance);
vault
.pause_with_role(&admin, &PauseReason::Maintenance)
.unwrap();
assert!(vault.is_paused());

vault.unpause_with_role(&admin);
Expand Down
3 changes: 3 additions & 0 deletions contracts/vault/src/formal_verification_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@

#![cfg(test)]

extern crate std;

use soroban_sdk::testutils::Address as _;
use soroban_sdk::{token, Address, Env};
use std::vec::Vec;

use crate::{YieldVault, YieldVaultClient};

Expand Down
Loading
Loading