commit 319dfdb56db2e5aa8f3e51378ca565314336dde7
parent 4bf4c4b1e262ae6cb30f77f7003a009d978d2db6
Author: Florian Dold <dold@taler.net>
Date: Fri, 24 Jul 2026 17:50:59 +0200
harness: reproduce SMS check loops
Diffstat:
5 files changed, 504 insertions(+), 6 deletions(-)
diff --git a/packages/taler-harness/src/harness/fake-challenger.ts b/packages/taler-harness/src/harness/fake-challenger.ts
@@ -14,7 +14,13 @@
GNU Taler; see the file COPYING. If not, see <http://www.gnu.org/licenses/>
*/
-import { AbsoluteTime, Duration, j2s, Logger } from "@gnu-taler/taler-util";
+import {
+ AbsoluteTime,
+ Duration,
+ j2s,
+ Logger,
+ TalerProtocolTimestamp,
+} from "@gnu-taler/taler-util";
import * as http from "node:http";
import { inflateSync } from "node:zlib";
import {
@@ -38,6 +44,14 @@ export interface TestfakeChallengerService {
export async function startFakeChallenger(options: {
port: number;
addressType: string;
+ /**
+ * "expires" reported by /info, i.e. how long the validated address stays
+ * valid. Real challenger computes this as last_tx_time +
+ * VALIDATION_EXPIRATION, so it can be much closer than the configured
+ * lifetime, or already past, for an address validated a while ago.
+ * Defaults to one day from now.
+ */
+ addressExpires?: TalerProtocolTimestamp;
}): Promise<TestfakeChallengerService> {
let nextNonceId = 1;
@@ -115,12 +129,14 @@ export async function startFakeChallenger(options: {
id: nonce,
address: myInfo.address,
address_type: addressType,
- expires: AbsoluteTime.toProtocolTimestamp(
- AbsoluteTime.addDuration(
- AbsoluteTime.now(),
- Duration.fromSpec({ days: 1 }),
+ expires:
+ options.addressExpires ??
+ AbsoluteTime.toProtocolTimestamp(
+ AbsoluteTime.addDuration(
+ AbsoluteTime.now(),
+ Duration.fromSpec({ days: 1 }),
+ ),
),
- ),
});
} else if (path.startsWith("/setup/")) {
let reqBody: string;
diff --git a/packages/taler-harness/src/harness/tops.ts b/packages/taler-harness/src/harness/tops.ts
@@ -588,6 +588,11 @@ export async function createTopsEnvironment(
* can tweak individual TOPS settings without forking the whole config.
*/
adjustExchangeConfig?: (config: Configuration) => void;
+ /**
+ * Extra/overriding environment for the exchange process. Merged over the
+ * TOPS defaults, so a test can e.g. vary the AML helper regexes.
+ */
+ extraProcEnv?: Record<string, string>;
} = {},
): Promise<TopsTestEnv> {
const db = await setupDb(t);
@@ -617,6 +622,7 @@ export async function createTopsEnvironment(
// "unbound variable" (exit 1) before it ever looks at its input.
// Value matches the staging (rusty) deployment.
EXCHANGE_AML_PROGRAM_TOPS_SMS_CHECK_REGEX: "\\+41[0-9]+",
+ ...(opts.extraProcEnv ?? {}),
},
});
diff --git a/packages/taler-harness/src/integrationtests/test-tops-aml-p2p-fresh-wallet.ts b/packages/taler-harness/src/integrationtests/test-tops-aml-p2p-fresh-wallet.ts
@@ -0,0 +1,239 @@
+/*
+ This file is part of GNU Taler
+ (C) 2026 Taler Systems S.A.
+
+ GNU Taler is free software; you can redistribute it and/or modify it under the
+ terms of the GNU General Public License as published by the Free Software
+ Foundation; either version 3, or (at your option) any later version.
+
+ GNU Taler is distributed in the hope that it will be useful, but WITHOUT ANY
+ WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+ A PARTICULAR PURPOSE. See the GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License along with
+ GNU Taler; see the file COPYING. If not, see <http://www.gnu.org/licenses/>
+ */
+
+/**
+ * Regression test: a rejected phone number re-requests the same check forever.
+ *
+ * A brand new wallet receives a p2p push payment and satisfies the resulting
+ * SMS check. Nothing has expired, no AML officer acted, and the reserve
+ * account has no legitimization_outcomes row at all, so
+ * TALER_EXCHANGEDB_current_rule_builder() correctly hands the AML program the
+ * default rules -- this is *not* the "Cannot iterate over null" defect.
+ *
+ * The problem is what tops-sms-check does when it decides the number is
+ * invalid (taler-exchange-helper-measure-tops-sms-check:110):
+ *
+ * NEW_RULES=$(echo "$CURRENT_RULES" | jq '.+{"to_investigate": true}')
+ *
+ * It returns the current rules *unchanged* and exits 0. So:
+ *
+ * - the program succeeded, hence no FALLBACK and the account is NOT frozen;
+ * - p2p-domestic-identification-requirement (THRESHOLD = CHF:0) is still in
+ * the rule set, so the exchange immediately asks for the very same SMS
+ * check again;
+ * - completing the check again reaches the same branch.
+ *
+ * The customer is stuck in a loop they cannot escape, and nothing about the
+ * account state signals a problem. A rejected check needs to either move the
+ * account to a measure that can terminate, or fail loudly -- silently
+ * re-imposing the rule that triggered it cannot converge.
+ *
+ * This test forces the reject branch by giving the exchange a regex that does
+ * not match the number, which is what happens if the "\\+41[0-9]+" in the
+ * ansible inventory ever reaches the helper without its backslash collapsing
+ * (YAML -> jinja -> systemd EnvironmentFile -> bash -> grep -E). The same
+ * loop results from any other reason the number is rejected, e.g. attributes
+ * that carry no CONTACT_PHONE, in which case PHONE_NUMBER is the string
+ * "null".
+ */
+
+/**
+ * Imports.
+ */
+import {
+ AbsoluteTime,
+ AccessToken,
+ AmountString,
+ Duration,
+ j2s,
+ Logger,
+ succeedOrThrow,
+ TalerExchangeHttpClient,
+ TransactionMajorState,
+ TransactionMinorState,
+ TransactionType,
+} from "@gnu-taler/taler-util";
+import { WalletApiOperation } from "@gnu-taler/taler-wallet-core";
+import {
+ createWalletDaemonWithClient,
+ withdrawViaBankV3,
+} from "../harness/environments.js";
+import { startFakeChallenger } from "../harness/fake-challenger.js";
+import {
+ GlobalTestState,
+ harnessHttpLib,
+ WalletClient,
+} from "../harness/harness.js";
+import {
+ createTopsEnvironment,
+ doFakeChallenger,
+ isFrozen,
+} from "../harness/tops.js";
+
+const logger = new Logger("test-tops-aml-p2p-fresh-wallet.ts");
+
+export async function runTopsAmlP2pFreshWalletTest(t: GlobalTestState) {
+ const { walletClient, bankClient, exchange, officerAcc } =
+ await createTopsEnvironment(t, {
+ // The un-collapsed form of the inventory's "\\+41[0-9]+": one or more
+ // literal backslashes followed by "41...", which no phone number
+ // contains. Every number is therefore rejected.
+ extraProcEnv: {
+ EXCHANGE_AML_PROGRAM_TOPS_SMS_CHECK_REGEX: "\\\\+41[0-9]+",
+ },
+ });
+
+ const challengerSms = await startFakeChallenger({
+ port: 6002,
+ addressType: "phone",
+ });
+
+ const exchangeClient = new TalerExchangeHttpClient(exchange.baseUrl, {
+ httpClient: harnessHttpLib,
+ });
+
+ // Sending wallet.
+ const w0 = await createWalletDaemonWithClient(t, { name: "w0" });
+
+ const wres = await withdrawViaBankV3(t, {
+ bankClient,
+ amount: "CHF:20",
+ exchange,
+ walletClient: w0.walletClient,
+ });
+ await wres.withdrawalFinishedCond;
+
+ const pushDebitRes = await doPeerPushDebit(t, {
+ walletClient: w0.walletClient,
+ amount: "CHF:10",
+ summary: "Test1",
+ });
+
+ // Receiving wallet: brand new reserve account.
+ const prepRes = await walletClient.call(
+ WalletApiOperation.PreparePeerPushCredit,
+ { talerUri: pushDebitRes.talerUri },
+ );
+ await walletClient.call(WalletApiOperation.ConfirmPeerPushCredit, {
+ transactionId: prepRes.transactionId,
+ });
+
+ // [kyc-rule-p2p-domestic-identification-requirement] has THRESHOLD = CHF:0,
+ // so any incoming p2p payment trips it.
+ await walletClient.call(WalletApiOperation.TestingWaitTransactionState, {
+ transactionId: prepRes.transactionId,
+ txState: {
+ major: TransactionMajorState.Pending,
+ minor: TransactionMinorState.KycRequired,
+ },
+ });
+
+ const txDet = await walletClient.call(WalletApiOperation.GetTransactionById, {
+ transactionId: prepRes.transactionId,
+ });
+ logger.info(`p2p credit tx: ${j2s(txDet)}`);
+
+ const accessToken = txDet.kycAccessToken;
+ t.assertTrue(!!accessToken);
+
+ const kycInfoResp = await exchangeClient.checkKycInfo(
+ accessToken as AccessToken,
+ );
+ logger.info(`kyc info: ${j2s(kycInfoResp)}`);
+ t.assertDeepEqual(kycInfoResp.case, "ok");
+
+ // NEXT_MEASURES = "sms-registration postal-registration", so the wallet
+ // picks one. Take the SMS one.
+ const smsReq = kycInfoResp.body.requirements.find((r) => r.form === "LINK");
+ t.assertTrue(smsReq != null);
+ const smsRequirementId = smsReq.id;
+ t.assertTrue(smsRequirementId != null);
+
+ t.logStep("complete-sms-check");
+
+ await doFakeChallenger(t, {
+ exchangeClient,
+ requirementId: smsRequirementId,
+ challenger: challengerSms,
+ address: { CONTACT_PHONE: "+41791234567" },
+ });
+
+ // The AML program exits 0, so nothing is frozen and nothing looks broken.
+ const decisionsResp = succeedOrThrow(
+ await exchangeClient.getAmlDecisions(officerAcc, { active: true }),
+ );
+ logger.info(`active decisions after SMS check: ${j2s(decisionsResp)}`);
+ const dec = decisionsResp.records[0];
+ t.assertTrue(dec != null);
+ t.assertTrue(!isFrozen(dec));
+
+ t.logStep("check-requirement-cleared");
+
+ // Expected: the SMS check has been satisfied, so the requirement is gone.
+ //
+ // Actual: tops-sms-check took the reject branch and echoed the rules back
+ // unchanged, so p2p-domestic-identification-requirement still applies and the
+ // exchange asks for the identical LINK check again -- with a fresh
+ // requirement id, i.e. a new instance of the same dead end.
+ const afterResp = await exchangeClient.checkKycInfo(accessToken as AccessToken);
+ logger.info(`kyc info after completing the check: ${j2s(afterResp)}`);
+ t.assertDeepEqual(afterResp.case, "ok");
+ const stillPending = afterResp.body.requirements.find(
+ (r) => r.form === "LINK",
+ );
+ t.assertTrue(stillPending == null);
+}
+
+async function doPeerPushDebit(
+ t: GlobalTestState,
+ args: {
+ walletClient: WalletClient;
+ amount: AmountString;
+ summary?: string;
+ },
+): Promise<{ transactionId: string; talerUri: string }> {
+ const purse_expiration = AbsoluteTime.toProtocolTimestamp(
+ AbsoluteTime.addDuration(AbsoluteTime.now(), Duration.fromSpec({ days: 2 })),
+ );
+ const initRet = await args.walletClient.call(
+ WalletApiOperation.InitiatePeerPushDebit,
+ {
+ partialContractTerms: {
+ amount: args.amount,
+ summary: args.summary ?? "Test P2P Payment",
+ purse_expiration,
+ },
+ },
+ );
+ await args.walletClient.call(WalletApiOperation.TestingWaitTransactionState, {
+ transactionId: initRet.transactionId,
+ txState: {
+ major: TransactionMajorState.Pending,
+ minor: TransactionMinorState.Ready,
+ },
+ });
+ const txDet = await args.walletClient.call(
+ WalletApiOperation.GetTransactionById,
+ { transactionId: initRet.transactionId },
+ );
+ t.assertTrue(txDet.type === TransactionType.PeerPushDebit);
+ const talerUri = txDet.talerUri;
+ t.assertTrue(!!talerUri);
+ return { transactionId: initRet.transactionId, talerUri };
+}
+
+runTopsAmlP2pFreshWalletTest.suites = ["wallet"];
+runTopsAmlP2pFreshWalletTest.timeoutMs = 120000;
diff --git a/packages/taler-harness/src/integrationtests/test-tops-aml-stale-validation.ts b/packages/taler-harness/src/integrationtests/test-tops-aml-stale-validation.ts
@@ -0,0 +1,233 @@
+/*
+ This file is part of GNU Taler
+ (C) 2026 Taler Systems S.A.
+
+ GNU Taler is free software; you can redistribute it and/or modify it under the
+ terms of the GNU General Public License as published by the Free Software
+ Foundation; either version 3, or (at your option) any later version.
+
+ GNU Taler is distributed in the hope that it will be useful, but WITHOUT ANY
+ WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+ A PARTICULAR PURPOSE. See the GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License along with
+ GNU Taler; see the file COPYING. If not, see <http://www.gnu.org/licenses/>
+ */
+
+/**
+ * Regression test: an *accepted* phone number can also loop forever.
+ *
+ * Same visible symptom as test-tops-aml-p2p-fresh-wallet (check re-requested
+ * endlessly, account never frozen), but reached without rejecting anything.
+ * The number matches the regex, tops-sms-check takes the success branch, and
+ * p2p-domestic-identification-requirement is correctly deleted.
+ *
+ * The problem is the expiry the program stamps on the rules it emits
+ * (taler-exchange-helper-measure-tops-sms-check:95):
+ *
+ * EXPIRATION_TIME=$(... '.context.expiration_time // .attributes.expires // ...')
+ *
+ * [kyc-measure-sms-registration] has CONTEXT = {}, so this is always
+ * .attributes.expires -- the challenger's address validation lifetime, which
+ * the exchange does not control. Real challenger stores
+ *
+ * address_expiration_time = LEAST($4, INT64_MAX - last_tx_time) + last_tx_time
+ *
+ * (challengerdb/do_insert_token.c), i.e. last_tx_time + VALIDATION_EXPIRATION.
+ * For an address validated a while ago that is much closer than the configured
+ * lifetime, and past it entirely once the validation has aged out.
+ *
+ * If that timestamp is already behind us, the rules are born expired:
+ *
+ * {"new_rules": {"expiration_time": {"t_s": <past>}, "rules": [], ...},
+ * "to_investigate": false}
+ *
+ * The next evaluation sees expired rules (begin_rule_update.c:554), finds no
+ * successor measure -- `del(..|nulls)` stripped the null one -- and
+ * run_measure() falls back to the default rules (begin_rule_update.c:341).
+ * The CHF:0 p2p rule is therefore back, and the same SMS check is requested
+ * again. The AML program exits 0 every time, so nothing is ever frozen and
+ * nothing in the account state records that anything went wrong.
+ *
+ * This is a defect in the exchange/program contract, not in the customer's
+ * input: an AML program is allowed to return a rule set that is already
+ * expired, and the exchange applies it.
+ */
+
+/**
+ * Imports.
+ */
+import {
+ AbsoluteTime,
+ AccessToken,
+ AmountString,
+ Duration,
+ j2s,
+ Logger,
+ succeedOrThrow,
+ TalerExchangeHttpClient,
+ TalerProtocolTimestamp,
+ TransactionMajorState,
+ TransactionMinorState,
+ TransactionType,
+} from "@gnu-taler/taler-util";
+import { WalletApiOperation } from "@gnu-taler/taler-wallet-core";
+import {
+ createWalletDaemonWithClient,
+ withdrawViaBankV3,
+} from "../harness/environments.js";
+import { startFakeChallenger } from "../harness/fake-challenger.js";
+import {
+ GlobalTestState,
+ harnessHttpLib,
+ WalletClient,
+} from "../harness/harness.js";
+import {
+ createTopsEnvironment,
+ doFakeChallenger,
+ isFrozen,
+} from "../harness/tops.js";
+
+const logger = new Logger("test-tops-aml-stale-validation.ts");
+
+export async function runTopsAmlStaleValidationTest(t: GlobalTestState) {
+ const { walletClient, bankClient, exchange, officerAcc } =
+ await createTopsEnvironment(t);
+
+ // An address whose validation has already aged out. The regex is the
+ // correct one, so the number itself is accepted.
+ const challengerSms = await startFakeChallenger({
+ port: 6002,
+ addressType: "phone",
+ addressExpires: AbsoluteTime.toProtocolTimestamp(
+ AbsoluteTime.subtractDuraction(
+ AbsoluteTime.now(),
+ Duration.fromSpec({ days: 1 }),
+ ),
+ ),
+ });
+
+ const exchangeClient = new TalerExchangeHttpClient(exchange.baseUrl, {
+ httpClient: harnessHttpLib,
+ });
+
+ const w0 = await createWalletDaemonWithClient(t, { name: "w0" });
+ const wres = await withdrawViaBankV3(t, {
+ bankClient,
+ amount: "CHF:20",
+ exchange,
+ walletClient: w0.walletClient,
+ });
+ await wres.withdrawalFinishedCond;
+
+ const pushDebitRes = await doPeerPushDebit(t, {
+ walletClient: w0.walletClient,
+ amount: "CHF:10",
+ summary: "Test1",
+ });
+
+ const prepRes = await walletClient.call(
+ WalletApiOperation.PreparePeerPushCredit,
+ { talerUri: pushDebitRes.talerUri },
+ );
+ await walletClient.call(WalletApiOperation.ConfirmPeerPushCredit, {
+ transactionId: prepRes.transactionId,
+ });
+ await walletClient.call(WalletApiOperation.TestingWaitTransactionState, {
+ transactionId: prepRes.transactionId,
+ txState: {
+ major: TransactionMajorState.Pending,
+ minor: TransactionMinorState.KycRequired,
+ },
+ });
+
+ const txDet = await walletClient.call(WalletApiOperation.GetTransactionById, {
+ transactionId: prepRes.transactionId,
+ });
+ const accessToken = txDet.kycAccessToken;
+ t.assertTrue(!!accessToken);
+
+ const kycInfoResp = await exchangeClient.checkKycInfo(
+ accessToken as AccessToken,
+ );
+ t.assertDeepEqual(kycInfoResp.case, "ok");
+ const smsReq = kycInfoResp.body.requirements.find((r) => r.form === "LINK");
+ t.assertTrue(smsReq != null);
+ const smsRequirementId = smsReq.id;
+ t.assertTrue(smsRequirementId != null);
+
+ t.logStep("complete-sms-check");
+
+ await doFakeChallenger(t, {
+ exchangeClient,
+ requirementId: smsRequirementId,
+ challenger: challengerSms,
+ address: { CONTACT_PHONE: "+41791234567" },
+ });
+
+ // The number was accepted, the program exited 0, nothing is frozen.
+ const decisionsResp = succeedOrThrow(
+ await exchangeClient.getAmlDecisions(officerAcc, { active: true }),
+ );
+ logger.info(`active decisions after SMS check: ${j2s(decisionsResp)}`);
+ const dec = decisionsResp.records[0];
+ t.assertTrue(dec != null);
+ t.assertTrue(!isFrozen(dec));
+
+ t.logStep("check-requirement-cleared");
+
+ // Expected: the check is satisfied and stays satisfied.
+ //
+ // Actual: the emitted rules were already expired, so the exchange reverts to
+ // the default rules and asks for the same SMS check again.
+ const afterResp = await exchangeClient.checkKycInfo(
+ accessToken as AccessToken,
+ );
+ logger.info(`kyc info after completing the check: ${j2s(afterResp)}`);
+ t.assertDeepEqual(afterResp.case, "ok");
+ const stillPending = afterResp.body.requirements.find(
+ (r) => r.form === "LINK",
+ );
+ t.assertTrue(stillPending == null);
+}
+
+async function doPeerPushDebit(
+ t: GlobalTestState,
+ args: {
+ walletClient: WalletClient;
+ amount: AmountString;
+ summary?: string;
+ },
+): Promise<{ transactionId: string; talerUri: string }> {
+ const purse_expiration = AbsoluteTime.toProtocolTimestamp(
+ AbsoluteTime.addDuration(AbsoluteTime.now(), Duration.fromSpec({ days: 2 })),
+ );
+ const initRet = await args.walletClient.call(
+ WalletApiOperation.InitiatePeerPushDebit,
+ {
+ partialContractTerms: {
+ amount: args.amount,
+ summary: args.summary ?? "Test P2P Payment",
+ purse_expiration,
+ },
+ },
+ );
+ await args.walletClient.call(WalletApiOperation.TestingWaitTransactionState, {
+ transactionId: initRet.transactionId,
+ txState: {
+ major: TransactionMajorState.Pending,
+ minor: TransactionMinorState.Ready,
+ },
+ });
+ const txDet = await args.walletClient.call(
+ WalletApiOperation.GetTransactionById,
+ { transactionId: initRet.transactionId },
+ );
+ t.assertTrue(txDet.type === TransactionType.PeerPushDebit);
+ const talerUri = txDet.talerUri;
+ t.assertTrue(!!talerUri);
+ return { transactionId: initRet.transactionId, talerUri };
+}
+
+runTopsAmlStaleValidationTest.suites = ["wallet"];
+runTopsAmlStaleValidationTest.timeoutMs = 120000;
diff --git a/packages/taler-harness/src/integrationtests/testrunner.ts b/packages/taler-harness/src/integrationtests/testrunner.ts
@@ -173,6 +173,8 @@ import { runTopsAmlKyxNaturalTest } from "./test-tops-aml-kyx-natural.js";
import { runTopsAmlLegiTest } from "./test-tops-aml-legi.js";
import { runTopsAmlExpiredSuccessorTest } from "./test-tops-aml-expired-successor.js";
import { runTopsAmlMeasuresTest } from "./test-tops-aml-measures.js";
+import { runTopsAmlP2pFreshWalletTest } from "./test-tops-aml-p2p-fresh-wallet.js";
+import { runTopsAmlStaleValidationTest } from "./test-tops-aml-stale-validation.js";
import { runTopsAmlNullCurrentRulesTest } from "./test-tops-aml-null-current-rules.js";
import { runTopsAmlPdfTest } from "./test-tops-aml-pdf.js";
import { runTopsChallengerTwiceTest } from "./test-tops-challenger-twice.js";
@@ -401,6 +403,8 @@ const allTests: TestMainFunction[] = [
runTopsAmlExpiredSuccessorTest,
runTopsAmlMeasuresTest,
runTopsAmlNullCurrentRulesTest,
+ runTopsAmlP2pFreshWalletTest,
+ runTopsAmlStaleValidationTest,
runTopsAmlLegiTest,
runTopsChallengerTwiceTest,
runKycFormBadMeasureTest,
