[29.x][VIES Integration] Per-environment daily request rate-limit

[dcenic]

What & why

Codeunit 248 ("VAT Lookup Ext. Data Hndl") calls the EU VIES service over SOAP to validate VAT registration numbers. VIES is unauthenticated and rate-limits by source IP. In Business Central online, many tenants share the same outbound egress IP per app service, so when a single noisy tenant floods VIES, VIES deny-lists the shared IP and every co-located tenant starts seeing "VIES service unavailable" errors.

We have a concrete example: one tenant scheduled a job that called codeunit 248 ~1.66 million times in 8 days, getting the whole app service deny-listed by VIES.

This PR adds a per-tenant (per-environment) daily request rate-limit to codeunit 248 so no single tenant can flood VIES and deny-list the shared outbound IP.

How it works

• At the start of OnRun (before the SOAP request), the codeunit reads a per-day counter, resets it when the UTC day changes, increments it, then persists and commits it before the outbound call.
• The counter lives in a Base Application table (243 "VAT Reg. No. Lookup Quota", DataPerCompany = false) holding a single environment-wide row. Because the read/modify/write executes in Base App code under a table lock, every caller shares one counter — including per-tenant extensions that call codeunit 248 (e.g. a custom "Verify All" action) and job-queue / API callers. Extensions cannot read or reset it.
• The cap is 2000 lookups/day — roughly 10x the 99th-percentile legitimate daily usage, and ~10x below the volume at which VIES deny-lists a shared IP.
• Because the counter is gated in OnRun, it covers all VIES code paths (interactive, background, API, direct CODEUNIT.Run(248), and codeunit 249 field validation, which funnels into 248).
• Online (SaaS) only. On-premises tenants own their own outbound IP and only affect themselves, so the limit does not apply there.

Enforcement

When a tenant reaches the daily cap, further lookups are blocked (Error(DailyQuotaExceededErr)) for the rest of the UTC day; the call that hits the limit also emits a security-audit entry and telemetry, and blocked lookups are not counted. The counter is stored in a dedicated table (243 "VAT Reg. No. Lookup Quota", Access = Internal), so the cap value or enforcement behavior can be adjusted later as a pure code change that can be serviced into release branches.

Why this replaces the earlier approach

This PR previously blocked codeunit 248 in background/API sessions. That was incomplete: a foreground "Verify All" over a large customer list (or a PTE action) still reaches VIES, and it would break legitimate low-volume automated callers. A per-environment daily quota is benign to legitimate users (well under the cap) while still stopping the bulk-flooding pattern from every session type.

Linked work

Fixes AB#651033

How I validated this

• I read the full diff and it contains only changes I intended.
• I added or updated tests for the new behavior.

What I tested and the outcome

New unit tests in ERM VAT VIES Lookup UT (codeunit 134193) exercise the quota decision logic directly (the check runs before the SOAP call, so no service/mock is needed). They use the Environment Info test library to simulate SaaS and internal test-only seams on codeunit 248 to drive the counter:

• DailyVIESCallQuotaBlocksWhenLimitReached — lookups beyond the daily limit are blocked and blocked lookups are not counted.
• DailyVIESCallQuotaResetsOnNewDay — after the day rolls over the counter resets, the customer gets a fresh full daily allowance, and the cap re-applies within the same day.
• DailyVIESCallQuotaSkippedOnPrem — the quota does not apply on-premises (nothing counted or blocked).

Risk & compatibility

• Online only — on-premises behavior is unchanged.
• The pre-existing background/API guard in codeunit 249 field validation is intentionally left in place — it is a complementary reliability control, not a duplicate of this quota.

[alexei-dobriansky]

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

This change adds a SaaS-only daily quota for EU VIES VAT registration number validation. The quota is shared across companies in the environment, stored in a new internal table, and checked before codeunit 248 sends the external SOAP request.

The quota direction matches the shared-egress problem, and the tests cover the limit, day rollover, and on-premises skip behavior. The current implementation still has unsafe boundaries: it commits inside the validation codeunit before later failures can occur, and it consumes quota before the code knows that a VIES request will actually be sent.

Problem-solution fit

Fit: Partial

The change targets the reported flood pattern with a per-environment daily cap, which is the right kind of control. The implementation is broader than the actual VIES-call path and can affect validation failures or handled calls that do not contact VIES.

Suggestions

S1 (🔴 High): Avoid committing before VIES can fail
Please do not call Commit() in the quota check before the VIES request. Codeunit 248 can still fail later, and this commit can make earlier caller work durable even though validation failed. Persist the quota without committing the caller transaction, or isolate only the quota update.

S2 (🟠 Moderate): Count only requests that can reach VIES
The quota is consumed before the IsHandled event and before an empty VAT number is rejected. A handled lookup or blank VAT error can use daily capacity without sending a request to VIES. Move the quota registration to the path after these checks, just before the SOAP request.

S3 (🟠 Moderate): Keep quota test hooks out of production API
These internal test helpers ship on the production codeunit and can change, seed, or clear the quota from any app with Base Application internals access. Prefer setting test data through the test app or another test-only seam that cannot change production quota state.

Risk assessment and necessity

Risk: The change is in a shared validation codeunit used by field validation, page actions, and direct codeunit calls. The daily counter is SaaS-only and the table lock limits races, but the early commit and broad entry-point placement can change validation and integration behavior outside real VIES calls.

Necessity: A per-environment quota is necessary because one environment can otherwise flood the unauthenticated VIES service and affect other environments that share the same outbound address. The scope is appropriate, but the transaction and placement issues should be fixed before merge.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11736 round=1 by=alexei-dobriansky at=2026-09-23T13:28:46.3690083Z lastSha=9f3f57ca444985b8c0d897416a68f64e0d4b5611 reviewKey=238d312f151c3de7fe080526814762fb6342e4b25c50ee98d0e3c78f183b1d0f suggestions=S1@0d5e9df1,S2@5385caf4,S3@593a59fc

[dcenic]

Thanks alexei-dobriansky — investigated. Note this branch is a line-for-line backport of #11734, so the substantive design points here really apply to the main PR; the fixes will land on #11734 and be mirrored to the backports.

S1 (Commit before the lookup can fail) — agree, will address on #11734. The Commit() in RegisterAndCheckVIESCallQuota is intentional: it releases the environment-wide quota row lock before the multi-second SOAP call (otherwise every VIES lookup in the environment serializes on that single row) and keeps the counter durable across a caller rollback. You're right, though, that committing the ambient transaction also makes a caller's pending writes durable and prevents rollback on a later validation failure. Fix: update the counter in an isolated write so only the quota row is committed, instead of committing the caller's transaction.

S2 (charge only requests that can reach VIES) — agree. Confirmed the quota is registered in OnRun before the OnRunOnBeforeLookupVatRegistrationFromWebService (IsHandled) event and before the empty-VAT-number check raised in LookupVatRegistrationFromWebService. Fix: move the registration into the standard VIES path, after the IsHandled decision and the blank-number check, so handled/replaced lookups and blank-number errors don't consume daily quota.

S3 (test hooks on the production codeunit) — by design. The *ForTest members are internal and reachable only by Microsoft test apps listed in the Base Application internalsVisibleTo (here, Tests-VAT); third-party extensions cannot call them. This follows the established internal test-seam pattern used across BaseApp and does not expose quota control to non-Microsoft code. Open to a different test-only seam if there's a preferred convention.

So: S1 and S2 are fair and will be fixed on the main PR (#11734) and ported here; treating them as design refinements rather than backport-specific defects.

[dcenic]

alexei-dobriansky Pushed a change addressing the review.

S2 - done. The quota is now charged only on the standard VIES request path: RegisterAndCheckVIESCallQuota was removed from OnRun, and the quota is invoked inside SendRequestToVatRegistrationService, after the blank-number check and only on the not-IsHandled path. Handled/replaced lookups and blank-number errors no longer consume daily quota.

S1 - done (dedicated codeunit run). The quota read-modify-write and its Commit moved into a new dedicated codeunit 247 "VAT Lookup Quota Mgt.", invoked via a codeunit run immediately before the outbound request, so the counter commit is its own unit of work and the increment stays durable.

One honest note on transaction semantics: a same-session codeunit run + Commit still commits the ambient transaction (AL has no partial commit; only a separate session would fully isolate, which is incompatible with synchronous blocking). In practice codeunit 248 already commits around the outbound call in its normal flow (the existing Commit in LookupVatRegistrationFromWebService), so this does not add a new ambient-commit point - it confines the quota commit to real VIES sends and makes it durable.

S3 - the quota test seams moved off the shared codeunit 248 onto the dedicated codeunit 247 and remain internal (reachable only by Microsoft test apps via internalsVisibleTo).

[dcenic]

Heads-up on the review state: the Good Sense Reviewer - Round 1 (Request Changes) above was raised against the first version of this work (blocking codeunit 248 in background/API sessions), before the per-environment daily VIES rate-limit was ported here. It no longer reflects the current code on this branch.

The current implementation matches main #11734: a SaaS-only, table-backed daily quota (table 243 "VAT Reg. No. Lookup Quota") charged on the standard VIES request path via a dedicated codeunit 247 "VAT Lookup Quota Mgt.". The design review (S1-S5) and the AL review agent findings were addressed on #11734 and mirrored here:

• Quota charged only on the standard path, after the blank-number check and the OnRun IsHandled event.
• Quota counter read-modify-write + commit moved into dedicated codeunit 247.
• Audit message reworded to say the daily limit was reached (not "blocked").
• Codeunit 247 set to Access = Internal; quota tests marked TransactionModel::AutoCommit.

[alexei-dobriansky]

Good Sense Reviewer - Round 2

Recommendation: Request Changes

What this PR does

The latest changes move quota charging to the standard VIES request path, after the blank-number and handled-request checks. They also move quota management into an internal codeunit and update the tests and audit wording.

The placement issue is fixed. However, running the quota logic in a dedicated codeunit does not create a separate transaction: its Commit() still commits the caller's pending writes before request preparation and the external call can fail.

Status of previous suggestions

ID	Title	Status	Author response
S1	Avoid committing before VIES can fail	Not addressed	The quota was moved to a dedicated codeunit, but the response confirms that its same-session Commit() still commits the ambient transaction.
S2	Count only requests that can reach VIES	Addressed	The quota now runs after the blank-number check and only on the standard, non-handled path.
S3	Keep quota test hooks out of production API	Disputed	The helpers are internal and limited to the allowlisted Microsoft test app, so tenant extensions cannot call them.

New observations (commits since round 1)

None - the new commits only address or respond to previous suggestions.

Risk assessment and necessity

Risk: The quota placement is now limited to real standard-path attempts, and the table lock protects concurrent increments. The remaining early Commit() can persist unrelated caller changes even when request preparation, an event subscriber, or the VIES request later fails; the existing commit happens later and can also be suppressed, so this is a new transaction boundary.

Necessity: The per-environment quota remains necessary to prevent one environment from exhausting a shared VIES egress address. The scope is appropriate, but the transaction boundary must be corrected before merge.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11736 round=2 by=alexei-dobriansky at=2026-09-23T19:21:11.173Z lastSha=4f8826434d87961465f9598ed9445c56eb9025ec reviewKey=ccf9ccf7b1b4439af080964589464a561b6e7823bd19b8976ac2a637329e239b suggestions=S1@0d5e9df1:notaddressed,S2@5385caf4:addressed,S3@593a59fc:disputed parentRound=1

[alexei-dobriansky]

Good Sense Reviewer - Round 3

Recommendation: Request Changes

What this PR does

The latest commit renames a telemetry label and resets quota-related test state during test initialization. It does not change the production quota flow.

The main quota design still matches the reported shared-egress problem, and the test cleanup is useful. The remaining blocker is unchanged: the quota code still commits before request preparation and the external VIES call can fail.

Status of previous suggestions

ID	Title	Status	Author response
S1	Avoid committing before VIES can fail	Not addressed	The latest commit does not change the production Commit() before request preparation and the external call.
S2	Count only requests that can reach VIES	Addressed	The quota still runs after the blank-number check and only on the standard request path.
S3	Keep quota test hooks out of production API	Disputed	No new code changes affect this point; the helpers remain internal and limited to the test app.

New observations (commits since round 2)

None - the new commit only renames telemetry text and resets test state.

Risk assessment and necessity

Risk: The latest commit lowers test leakage risk, but it does not reduce the production transaction risk. The early Commit() can make caller changes durable even if request preparation, an event subscriber, or the VIES request later fails.

Necessity: The quota is still needed to stop one environment from exhausting the shared VIES capacity. The scope is right, but the transaction boundary must be corrected before merge.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11736 round=3 by=alexei-dobriansky at=2026-09-24T01:05:06.783Z lastSha=3bc6292f2c99c6678616c0100430781e02ede959 reviewKey=7ace5d67fd7adb0e448cc755ec5df970c526097f1daa04104fb054ef127b1c79 suggestions=S1@0d5e9df1:notaddressed,S2@5385caf4:addressed,S3@593a59fc:disputed parentRound=2