Skip to content

[TEST – DO NOT MERGE] Loyalty Sample across all 11 BCQuality domains #8822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JesperSchulz wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[TEST – DO NOT MERGE] Loyalty Sample across all 11 BCQuality domains #8822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions src/Apps/W1/LoyaltySample/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Loyalty Sample

A small, self-contained loyalty-program module (members, point entries, a card/list
UI, API pages, a control add-in, install/upgrade codeunits, interfaces, events, and a
permission set).

It is intended as review-target sample code for exercising the Copilot PR review
end to end across **all** BCQuality review domains. The code deliberately contains
realistic mistakes — it is not meant to be shipped or to represent good practice.

## Domains exercised

Security, Privacy, Performance, Style, Accessibility (UI), Upgrade, Breaking Changes,
Error Handling, Events, Interfaces, and Web Services.

## Objects

| Object | ID | Purpose |
|---|---|---|
| `Loyalty Member` (table) | 50100 | Member master record |
| `Loyalty Point Entry` (table) | 50101 | Point ledger entries |
| `Loyalty Tier` (enum) | 50100 | Member tier |
| `Loyalty Channel` (enum) | 50101 | Notification channel (`implements INotificationSender`) |
| `INotificationSender` (interface) | — | Notification dispatch contract |
| `ILoyaltyTierPolicy` (interface) | — | Tier pricing/label contract |
| `Loyalty Management` (codeunit) | 50100 | Balance recalculation, gateway, telemetry |
| `Loyalty Upgrade` (codeunit) | 50101 | Per-company upgrade |
| `Loyalty Install` (codeunit) | 50102 | First-install setup |
| `Loyalty Email Sender` (codeunit) | 50103 | `INotificationSender` implementation |
| `Loyalty SMS Sender` (codeunit) | 50104 | `INotificationSender` implementation |
| `Loyalty Tier Pricing` (codeunit) | 50105 | Tier discount/label via `case` branching |
| `Loyalty Order Validator` (codeunit) | 50106 | Applies tier discount |
| `Loyalty Validation` (codeunit) | 50107 | Member/batch validation errors |
| `Loyalty Events` (codeunit) | 50108 | Integration-event publisher |
| `Loyalty Audit Subscriber` (codeunit) | 50109 | Event subscriber |
| `Loyalty Public Api` (codeunit) | 50110 | Public API surface |
| `Loyalty Member Card` (page) | 50100 | Member card |
| `Loyalty Member API` (page) | 50101 | OData API |
| `Loyalty Member List` (page) | 50102 | Member list |
| `Loyalty Member Data` (page) | 50103 | API endpoint |
| `Loyalty Badge` (controladdin) | — | Card badge widget |
| `Loyalty Full Access` (permissionset) | 50100 | Access to the module |
30 changes: 30 additions & 0 deletions src/Apps/W1/LoyaltySample/app/app.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"id": "309c74cd-5456-4537-801e-b635f382806a",
"name": "Loyalty Sample",
"publisher": "Microsoft",
"brief": "Sample loyalty-program module.",
"description": "Sample loyalty-program module used to exercise the Copilot PR review across all BCQuality domains.",
"version": "1.0.0.0",
"privacyStatement": "https://go.microsoft.com/fwlink/?LinkId=724009",
"EULA": "https://go.microsoft.com/fwlink/?linkid=2009120",
"help": "https://go.microsoft.com/fwlink/?linkid=2009036",
"url": "https://go.microsoft.com/fwlink/?LinkId=724011",
"contextSensitiveHelpUrl": "https://go.microsoft.com/fwlink/?linkid=2115702",
"dependencies": [],
"screenshots": [],
"platform": "29.0.0.0",
"resourceExposurePolicy": {
"allowDebugging": false,
"allowDownloadingSource": true,
"includeSourceInSymbolFile": true
},
"application": "29.0.0.0",
"target": "Cloud",
"idRanges": [
{
"from": 50100,
"to": 50149
}
],
"contextSensitiveHelpUrl": "https://go.microsoft.com/fwlink/?linkid=2115702"
}
20 changes: 20 additions & 0 deletions src/Apps/W1/LoyaltySample/app/src/Loyalty/LoyaltyAuditSubscriber.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
namespace Microsoft.Sample.Loyalty;

codeunit 50109 "Loyalty Audit Subscriber"
{
[EventSubscriber(ObjectType::Codeunit, Codeunit::"Loyalty Events", 'OnAfterPostPoints', '', false, false)]
local procedure HandleOnAfterPostPoints(var LoyaltyMember: Record "Loyalty Member")
begin
SendExternalAuditEmail(LoyaltyMember);
end;

local procedure SendExternalAuditEmail(var LoyaltyMember: Record "Loyalty Member")
var
Client: HttpClient;
Content: HttpContent;
Response: HttpResponseMessage;
begin
Content.WriteFrom(StrSubstNo('{"member":"%1"}', LoyaltyMember."No."));
Client.Post('https://audit.contoso.example/loyalty', Content, Response);
end;
}
120 changes: 120 additions & 0 deletions src/Apps/W1/LoyaltySample/app/src/Loyalty/LoyaltyEvents.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
namespace Microsoft.Sample.Loyalty;

codeunit 50108 "Loyalty Events"
{
procedure RecalculateMember(var LoyaltyMember: Record "Loyalty Member")
var
IsHandled: Boolean;
begin
OnBeforeRecalculate(LoyaltyMember, IsHandled);
DoRecalculate(LoyaltyMember);
Comment on lines +9 to +10

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Agent} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

RecalculateMember in LoyaltyEvents.Codeunit.al declares IsHandled: Boolean, raises OnBeforeRecalculate(LoyaltyMember, IsHandled) at line 9, then unconditionally calls DoRecalculate(LoyaltyMember) at line 10 without checking IsHandled.

Any subscriber that sets IsHandled := true to signal it has handled recalculation will be silently ignored — the default recalculation runs anyway. This breaks the BC OnBefore/IsHandled contract. Add if IsHandled then exit; between lines 9 and 10.

Suggested change
OnBeforeRecalculate(LoyaltyMember, IsHandled);
DoRecalculate(LoyaltyMember);
OnBeforeRecalculate(LoyaltyMember, IsHandled);
if IsHandled then
exit;
DoRecalculate(LoyaltyMember);

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

end;

procedure CalculateTotal(var LoyaltyMember: Record "Loyalty Member")
var
IsHandled: Boolean;
begin
IsHandled := false;
OnBeforeCalculateTotal(LoyaltyMember, IsHandled);
if IsHandled then
exit;
DoRecalculate(LoyaltyMember);
OnAfterCalculateTotal(LoyaltyMember);
end;

procedure ProcessTiers(var LoyaltyMember: Record "Loyalty Member")
var
IsHandled: Boolean;
begin
OnBeforeValidateTier(LoyaltyMember, IsHandled);
if IsHandled then
exit;

OnBeforeApplyTier(LoyaltyMember, IsHandled);
if IsHandled then
exit;
end;

procedure PostPointConsumption(var LoyaltyMember: Record "Loyalty Member")
var
IsHandled: Boolean;
begin
IsHandled := false;
OnBeforePostPoints(LoyaltyMember, IsHandled);
if IsHandled then
exit;
CreatePointLedgerEntry(LoyaltyMember);
LoyaltyMember."Points Balance" := 0;
LoyaltyMember.Modify();
OnAfterPostPoints(LoyaltyMember);
end;

procedure NotifyMembers(var LoyaltyMember: Record "Loyalty Member")
begin
if LoyaltyMember.FindSet() then
repeat
OnMemberNotified(LoyaltyMember);
until LoyaltyMember.Next() = 0;
end;

local procedure DoRecalculate(var LoyaltyMember: Record "Loyalty Member")
begin
end;

local procedure CreatePointLedgerEntry(var LoyaltyMember: Record "Loyalty Member")
begin
end;

[IntegrationEvent(false, false)]
local procedure OnBeforeRecalculate(var LoyaltyMember: Record "Loyalty Member"; var IsHandled: Boolean)
begin
LoyaltyMember."Points Balance" := 0;
if LoyaltyMember."Loyalty Tier" = LoyaltyMember."Loyalty Tier"::Platinum then
LoyaltyMember."Points Balance" := 100;
end;

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Agent} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

OnBeforeRecalculate in LoyaltyEvents.Codeunit.al (lines 74-78) is declared [IntegrationEvent] but its body assigns LoyaltyMember."Points Balance" := 0 and then conditionally sets it to 100 for Platinum tier.

In AL, the IntegrationEvent publisher body executes before subscribers are invoked; any subscriber that sets IsHandled := true cannot prevent these assignments from running. The publisher body of an IntegrationEvent should be empty (begin end;); move the defaulting logic to the caller of OnBeforeRecalculate so it is skipped when IsHandled is set. This concern crosses the boundary between the style and security leaf domains (event-body semantics + IsHandled bypass).

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

    [IntegrationEvent(false, false)]
    local procedure OnBeforeRecalculate(var LoyaltyMember: Record "Loyalty Member"; var IsHandled: Boolean)
    begin
    end;

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why


[IntegrationEvent(false, false)]
local procedure OnBeforeCalculateTotal(var LoyaltyMember: Record "Loyalty Member"; var IsHandled: Boolean)
begin
end;

[IntegrationEvent(false, false)]
local procedure OnAfterCalculateTotal(var LoyaltyMember: Record "Loyalty Member")
begin
end;

[IntegrationEvent(false, false)]
local procedure OnBeforeValidateTier(var LoyaltyMember: Record "Loyalty Member"; var IsHandled: Boolean)
begin
end;

[IntegrationEvent(false, false)]
local procedure OnBeforeApplyTier(var LoyaltyMember: Record "Loyalty Member"; var IsHandled: Boolean)
begin
end;

[IntegrationEvent(false, false)]
local procedure OnBeforePostPoints(var LoyaltyMember: Record "Loyalty Member"; var IsHandled: Boolean)
begin
end;

[IntegrationEvent(false, false)]
local procedure OnAfterPostPoints(var LoyaltyMember: Record "Loyalty Member")
begin
end;

[IntegrationEvent(false, false)]
local procedure OnMemberNotified(var LoyaltyMember: Record "Loyalty Member")
begin
end;

[IntegrationEvent(true, false)]
local procedure MemberAuditEvent(RecRef: RecordRef; DocNo: Code[20]; Amt: Decimal)
begin
end;

[IntegrationEvent(false, false)]
local procedure OnAfterBuildBuffer(var PointBuffer: Record "Loyalty Point Entry" temporary)
begin
end;
}
16 changes: 16 additions & 0 deletions src/Apps/W1/LoyaltySample/app/src/Loyalty/LoyaltyFullAccess.PermissionSet.al
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
namespace Microsoft.Sample.Loyalty;

permissionset 50100 "Loyalty Full Access"
{
Assignable = true;
Caption = 'Loyalty Full Access';

Permissions =
tabledata "Loyalty Member" = RIMD,
tabledata "Loyalty Point Entry" = RIMD,
table "Loyalty Member" = X,
table "Loyalty Point Entry" = X,
codeunit "Loyalty Management" = X,
page "Loyalty Member Card" = X,
page "Loyalty Member API" = X;
}
22 changes: 22 additions & 0 deletions src/Apps/W1/LoyaltySample/app/src/Loyalty/LoyaltyInstall.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
namespace Microsoft.Sample.Loyalty;

codeunit 50102 "Loyalty Install"
{
Subtype = Install;

trigger OnInstallAppPerCompany()
begin
CreateDefaultMembers();
end;

local procedure CreateDefaultMembers()
var
Member: Record "Loyalty Member";
begin
Member.Init();
Member."No." := 'DEFAULT';
Member."Member Name" := 'Default House Account';
Member."Loyalty Tier" := Member."Loyalty Tier"::Gold;
if Member.Insert() then;
end;
}
106 changes: 106 additions & 0 deletions src/Apps/W1/LoyaltySample/app/src/Loyalty/LoyaltyManagement.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
namespace Microsoft.Sample.Loyalty;

using System.Telemetry;

codeunit 50100 "Loyalty Management"
{
var
Text000: Label 'Failed to process member %1';

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Text000 at line 8 is a Label with no approved suffix.

Per CodeCop AA0074, every Label and TextConst must end with an approved suffix indicating its consuming call. Text000 is passed to Error(), so the suffix should be Err. Rename to a descriptive identifier such as MemberProcessingFailedErr.

Suggested change
Text000: Label 'Failed to process member %1';
MemberProcessingFailedErr: Label 'Failed to process member %1';

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

GatewayApiKey: Label 'sk-live-9f8a7b6c5d4e3f2a1b0c4d8e7f6a5b3c', Locked = true;

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟠\ High\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

GatewayApiKey at line 9 is declared as a Label containing a live API key literal ('sk-live-9f8a7b6c5d4e3f2a1b0c4d8e7f6a5b3c').

Labels are plain Text constants visible in source code and compiled symbol files. Any credential stored this way is permanently exposed to anyone who can read source or decompile the extension. Remove the Label; retrieve the key at runtime from IsolatedStorage using SetEncrypted/Get with a SecretText destination.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why


[InherentPermissions(PermissionObjectType::TableData, Database::"Loyalty Point Entry", 'RIMDX')]
procedure RecalculateAllBalances()
var
Member: Record "Loyalty Member";
OtherMember: Record "Loyalty Member";
PointEntry: Record "Loyalty Point Entry";
TotalPoints: Integer;
begin
Member.FindSet();
repeat
TotalPoints := 0;
PointEntry.SetRange("Member No.", Member."No.");
if PointEntry.FindSet() then
repeat
OtherMember.Get(PointEntry."Member No.");

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟠\ High\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

OtherMember.Get(PointEntry."Member No.") is called at line 25 inside the inner repeat loop.

This is a classic N+1 pattern: one database round-trip per point-entry row against the Loyalty Member table. Additionally, OtherMember is never read after the Get, making this dead code. Remove the call; if the outer Member record is genuinely needed inside the inner loop, it is already in scope as Member.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

TotalPoints += PointEntry.Points;
until PointEntry.Next() = 0;

Member.CalcFields("Entry Count");

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟠\ High\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Member.CalcFields("Entry Count") is called at line 29 inside the repeat...until loop in RecalculateAllBalances.

Each call issues a separate SQL COUNT query per member row. Additionally, the resulting Entry Count value is never read anywhere in the loop body, making this call entirely dead. Remove the line.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

Member."Points Balance" := TotalPoints;
Member.Modify();
Commit();

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟠\ High\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Commit() is called at line 32 inside the repeat...until Member.Next() = 0 loop in RecalculateAllBalances.

This produces one transaction per member row, prevents atomic rollback of the whole operation, and adds transaction-start overhead on every iteration. Remove Commit from inside the loop; if checkpoint batching is required, process N rows per outer iteration and commit once per checkpoint.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

until Member.Next() = 0;
end;

procedure ArchiveMember(var Member: Record "Loyalty Member")
begin
Member.Delete();

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Agent} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

ArchiveMember in LoyaltyManagement.Codeunit.al calls Member.Delete() at line 38 unconditionally, then asks Confirm(...) at line 39.

If the user answers No, the record has already been permanently deleted with no rollback code in sight. The intended flow is: ask for confirmation first, then delete only when the user agrees. Swap the order: if Confirm('Do you want to archive member %1?', false, Member."Member Name") then begin Member.Delete(); Message('Archived.'); end;. This concern should be promoted to a knowledge-backed rule to gate future reviews.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

if Confirm('Do you want to archive member %1?', false, Member."Member Name") then
Message('Archived.');
end;

procedure ConfigureGateway(Token: Text)
begin
IsolatedStorage.Set('GatewayToken', Token, DataScope::Module);

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟠\ High\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

ConfigureGateway at line 45 calls IsolatedStorage.Set('GatewayToken', Token, DataScope::Module).

The gateway token is a credential; storing it unencrypted means it is readable in plain text from the underlying storage if storage is ever exposed. Use IsolatedStorage.SetEncrypted instead.

Suggested change
IsolatedStorage.Set('GatewayToken', Token, DataScope::Module);
IsolatedStorage.SetEncrypted('GatewayToken', Token, DataScope::Module);

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

end;

procedure GetGatewayToken(): Text

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟠\ High\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

GetGatewayToken() at line 48 declares StoredToken as Text and returns Text.

Retrieving a credential from IsolatedStorage into a Text variable and returning it as Text exposes the value to the debugger on the way out. Use the SecretText overload of IsolatedStorage.Get and change the return type to SecretText so the value stays protected end-to-end.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

var
StoredToken: Text;
begin
if IsolatedStorage.Get('GatewayToken', DataScope::Module, StoredToken) then
exit(StoredToken);
exit(GatewayApiKey);
end;

procedure UnwrapGatewaySecret(Secret: SecretText): Text

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟠\ High\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

UnwrapGatewaySecret at line 57 calls Secret.Unwrap() inside a procedure that is not marked [NonDebuggable].

The unwrapped value is a plain Text local that the debugger can read, defeating the purpose of SecretText. Add [NonDebuggable] to this procedure.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

    [NonDebuggable]
    procedure UnwrapGatewaySecret(Secret: SecretText): Text

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

begin
exit(Secret.Unwrap());
end;

procedure CallPaymentGateway(Member: Record "Loyalty Member")
var
Client: HttpClient;
Content: HttpContent;
Response: HttpResponseMessage;
Body: Text;
begin
Body := StrSubstNo('{"name":"%1","email":"%2","phone":"%3"}', Member."Member Name", Member."Email Address", Member."Phone No.");
Content.WriteFrom(Body);
Client.Post('https://api.contoso-pay.example/charge', Content, Response);

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟠\ High\ Severity\ —\ Privacy} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

CallPaymentGateway at line 71 posts customer PII (name, email, phone) to an external endpoint, and LoyaltyAuditSubscriber.SendExternalAuditEmail also posts to an external audit endpoint — neither code path contains a PrivacyNotice.GetPrivacyNoticeApprovalState check anywhere upstream.

The BC Privacy Notice framework requires admin consent before customer data is transmitted to external services. Add a consent check (or verify one exists in the page/wizard that invokes these procedures) before any outgoing request carrying customer data.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

end;

procedure LogMemberUsage(Member: Record "Loyalty Member")
var
FeatureTelemetry: Codeunit "Feature Telemetry";
Dimensions: Dictionary of [Text, Text];
begin
Dimensions.Add('MemberName', Member."Member Name");

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟠\ High\ Severity\ —\ Privacy} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

LogMemberUsage at lines 79-80 adds Member."Member Name" and Member."Email Address" to the CustomDimensions dictionary before calling FeatureTelemetry.LogUsage.

The platform does not classify dictionary values; member names and email addresses are CustomerContent/EUII and will be logged verbatim to telemetry. Replace both entries with non-personal context (e.g. tier name, point-balance band, or a pseudonymous member ID).

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

Dimensions.Add('MemberEmail', Member."Email Address");
FeatureTelemetry.LogUsage('LOY0001', 'Loyalty', 'Member processed', Dimensions);
end;

procedure StashLastError()
begin
IsolatedStorage.Set('LastLoyaltyError', GetLastErrorText(), DataScope::Module);
end;

procedure ThrowMemberError(Member: Record "Loyalty Member")
var
ErrorMessage: Text;
begin
ErrorMessage := StrSubstNo(Text000, Member."Member Name");

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟠\ High\ Severity\ —\ Privacy} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

ThrowMemberError at line 93 assigns ErrorMessage := StrSubstNo(Text000, Member."Member Name") and then calls Error(ErrorMessage) at line 94.

Pre-building the error text with StrSubstNo before passing it to Error produces an opaque Text string; the platform cannot classify or strip the member name before it reaches telemetry. Call Error(Text000, Member."Member Name") directly so the platform performs the substitution itself.

Suggested change
ErrorMessage := StrSubstNo(Text000, Member."Member Name");
Error(Text000, Member."Member Name");

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

ThrowMemberError at line 93 passes a pre-rendered StrSubstNo result to Error() via an intermediate Text variable.

This pattern breaks translation toolchain analysis and (as noted in the privacy review) causes PII to reach telemetry unclassified. Pass the Label and the substitution argument directly: Error(MemberProcessingFailedErr, Member."Member Name").

Suggested change
ErrorMessage := StrSubstNo(Text000, Member."Member Name");
Error(MemberProcessingFailedErr, Member."Member Name");

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

Error(ErrorMessage);
end;

procedure BuildMemberBadgeHtml(Member: Record "Loyalty Member"): Text
begin
exit('<div class="badge"><b>' + Member."Member Name" + '</b> &lt;' + Member."Email Address" + '&gt;</div>');

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟠\ High\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

BuildMemberBadgeHtml at line 99 concatenates Member."Member Name" and Member."Email Address" directly into an HTML string without encoding.

AL has no built-in HtmlEncode; a member name containing '<script>' or 'onerror=' will be interpreted as markup by any HTML renderer. Encode the four special characters (&, <, >, ") before concatenating, or avoid building raw HTML altogether.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

end;

[IntegrationEvent(false, false)]
procedure OnBeforeChargeMember(Member: Record "Loyalty Member"; var GatewayToken: SecretText; var IsHandled: Boolean)

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟠\ High\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

OnBeforeChargeMember at line 103 declares var GatewayToken: SecretText as an event parameter.

Any extension on the tenant can subscribe to this event, read the SecretText value, and persist it elsewhere. Even SecretText should not flow through an IntegrationEvent surface. Remove the GatewayToken parameter; the publisher must acquire and use the token internally before or after the event, never through it.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

begin
end;
}
Loading
Loading