fix(scorechain): drop code default for risk threshold — fail loud instead of defaulting#4103
Merged
Merged
Conversation
Collaborator
Author
|
Completed in 2 review passes, each running an independent conformity review and a logic/security review in parallel — 0 issues on both passes:
The value is provided as an env var by the deployment environment; prod + dev already set it, so no throw in real environments. All CI checks green. (Branch history was rewritten to keep infrastructure/deployment details out of this public repo; content is otherwise unchanged.) |
e999198 to
179ebef
Compare
…tead of defaulting The Scorechain risk threshold has no code fallback anymore; the value comes solely from the SCORECHAIN_RISK_THRESHOLD env var. - config: unset => undefined (was `?? 70`). - isHighRisk: throw if the threshold is null/NaN. A missing value must not silently disable the gate (`riskScore < undefined/NaN` is always false); the throw is caught by the screenScorechain callers and routes the tx to manual review (fail-closed + loud). - test: the screening spec sets the threshold explicitly (70) so the score-vs-threshold assertions stay deterministic without an ambient default. - docs: drop the now-stale "(default 70)" from the spec.
179ebef to
2bdcb98
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes the code fallback for the Scorechain risk threshold. The value now comes solely from the
SCORECHAIN_RISK_THRESHOLDenv var — no code default — so a missing value fails loud instead of silently defaulting.Changes
config.ts):SCORECHAIN_RISK_THRESHOLDunset →undefined(was?? 70).riskThreshold: number | undefined.isHighRisk(scorechain-screening.service.ts): throws if the threshold isnull/NaN. A missing value must not silently disable the gate (riskScore < undefined/NaNis alwaysfalse). The throw is caught by thescreenScorechaincallers (buy-crypto / buy-fiat preparation) and routes the tx to manual review — fail-closed + loud.scorechain-screening.service.spec.ts): setsSCORECHAIN_RISK_THRESHOLD=70inbeforeAll(restored inafterAll) so the score-vs-threshold assertions stay deterministic. Only this spec calls the realisHighRisk; the buy-crypto/buy-fiat preparation specs mock it.Scope / safety
riskThresholdis read only byisHighRisk, so no other consumer sees the widened type.isHighRiskis unreachable withoutSCORECHAIN_API_KEY(thescreenScorechaingate returns early), so a local run without the threshold does not throw.