mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 09:26:52 +01:00
Fix lifecycle privacy review findings
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a26cd6d6-ff49-433e-bc53-f645c455ebdd
This commit is contained in:
parent
0c2a0ceb82
commit
925beb3204
5 changed files with 12 additions and 14 deletions
|
|
@ -1,13 +1,12 @@
|
|||
codeunit 50308 "ErrorInfo Privacy Bad"
|
||||
{
|
||||
procedure RaiseSynchronizationError(Customer: Record Customer; ResponseBody: Text)
|
||||
procedure RaiseSynchronizationError(Customer: Record Customer)
|
||||
var
|
||||
FailureInfo: ErrorInfo;
|
||||
begin
|
||||
FailureInfo.Message := StrSubstNo('Synchronization failed for %1.', Customer."E-Mail");
|
||||
FailureInfo.DataClassification := DataClassification::SystemMetadata;
|
||||
FailureInfo.ErrorType := ErrorType::Internal;
|
||||
FailureInfo.DetailedMessage := ResponseBody;
|
||||
Error(FailureInfo);
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,11 +7,9 @@ codeunit 50307 "ErrorInfo Privacy Good"
|
|||
FailureInfo.Message := SynchronizationFailedErr;
|
||||
FailureInfo.DataClassification := DataClassification::SystemMetadata;
|
||||
FailureInfo.ErrorType := ErrorType::Client;
|
||||
FailureInfo.DetailedMessage := RetryDiagnosticsTxt;
|
||||
Error(FailureInfo);
|
||||
end;
|
||||
|
||||
var
|
||||
RetryDiagnosticsTxt: Label 'The remote service rejected the request. Review the integration telemetry event.';
|
||||
SynchronizationFailedErr: Label 'The synchronization could not be completed.';
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [19..]
|
||||
bc-version: [14..]
|
||||
domain: privacy
|
||||
keywords: [errorinfo, message, dataclassification, errortype, detailedmessage, copy-details, telemetry]
|
||||
technologies: [al]
|
||||
|
|
@ -11,16 +11,16 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
`ErrorInfo.Message` is sent to telemetry; with `ErrorType::Client` it is also the primary client message, while `ErrorType::Internal` replaces it in the client with a generic message but still sends the specified text to telemetry. `DataClassification` classifies the content in `Message`; it does not make incorrectly classified personal data safe. `DetailedMessage`, available from BC 19, is omitted from the primary message but is included in the error dialog's **Copy details** content.
|
||||
`ErrorInfo.Message` is sent to telemetry; with `ErrorType::Client` it is also the primary client message, while `ErrorType::Internal` replaces it in the client with a generic message but still sends the specified text to telemetry. `DataClassification` classifies the content in `Message`; it does not make incorrectly classified personal data safe. In BC 19 and later, `DetailedMessage` is omitted from the primary message but is included in the error dialog's **Copy details** content.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Keep `Message` stable and classify its actual content. Choose `ErrorType` for client usability, not as a telemetry privacy boundary. Put only support-safe technical context in `DetailedMessage`, because a user can copy it from the dialog.
|
||||
Keep `Message` stable and classify its actual content. Choose `ErrorType` for client usability, not as a telemetry privacy boundary. In BC 19 and later, put only support-safe technical context in `DetailedMessage`, because a user can copy it from the dialog. The samples use only members available at the BC 14 article floor.
|
||||
|
||||
See sample: `errorinfo-telemetry-classification-and-errortype.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Marking a dynamic customer-bearing `Message` as `SystemMetadata`, assuming `ErrorType::Internal` keeps it out of telemetry, or placing secrets and personal data in `DetailedMessage` because it is not the primary dialog text.
|
||||
Marking a dynamic customer-bearing `Message` as `SystemMetadata`, assuming `ErrorType::Internal` keeps it out of telemetry, or, in BC 19 and later, placing secrets and personal data in `DetailedMessage` because it is not the primary dialog text.
|
||||
|
||||
See sample: `errorinfo-telemetry-classification-and-errortype.bad.al`.
|
||||
|
|
|
|||
|
|
@ -39,10 +39,10 @@ Narrow the relevant files to the subset that applies to the changes under review
|
|||
|
||||
- The changed AL object names and types — especially tables and tableextensions (for `DataClassification` on fields), codeunits that call `Error`, `Session.LogMessage`, or `FeatureTelemetry`, codeunits performing outgoing HTTP requests with customer data, migration codeunits, and objects reading or writing `IsolatedStorage`.
|
||||
- The changed procedures and triggers, weighted toward those that call `Error`, construct `ErrorInfo`, call `Session.LogMessage`, `StrSubstNo`, `GetLastErrorText`/`GetLastErrorCallStack`, `FeatureTelemetry.LogUsage`/`LogUptake`/`LogError`, `HttpClient.Post`/`Get`, `IsolatedStorage.Set`/`SetEncrypted`/`Get`, or `PrivacyNotice.GetPrivacyNoticeApprovalState`.
|
||||
- Tokens extracted from the diff that relate to privacy (`DataClassification`, `CustomerContent`, `EndUserIdentifiableInformation`, `EndUserPseudonymousIdentifiers`, `SystemMetadata`, `ToBeClassified`, `PrivacyNotice`, `ErrorInfo`, `Message`, `DetailedMessage`, `ErrorType`, `Internal`, `Client`, `GetLastErrorText`, `GetLastErrorCallStack`, `TelemetryScope`, `FeatureTelemetry`, `CustomDimensions`, `LogUsage`, `LogUptake`, `LogError`, `ErrorText`, `ErrorCallStack`, `alErrorText`, `alErrorCallStack`, `HybridSL`, `HybridGP`, `HybridBC`).
|
||||
- Worklist ErrorInfo privacy guidance when any of `Message`, `DataClassification`, `ErrorType`, or `DetailedMessage` changes. For every `FeatureTelemetry.LogError`, inspect the dedicated error text and call-stack arguments in addition to explicit custom dimensions.
|
||||
- Tokens extracted from the diff that relate to privacy (`DataClassification`, `CustomerContent`, `EndUserIdentifiableInformation`, `EndUserPseudonymousIdentifiers`, `SystemMetadata`, `ToBeClassified`, `PrivacyNotice`, `ErrorInfo`, qualified ErrorInfo member access, `Internal`, `Client`, `GetLastErrorText`, `GetLastErrorCallStack`, `TelemetryScope`, `FeatureTelemetry`, `CustomDimensions`, `LogUsage`, `LogUptake`, `LogError`, `ErrorText`, `ErrorCallStack`, `alErrorText`, `alErrorCallStack`, `HybridSL`, `HybridGP`, `HybridBC`).
|
||||
- Worklist ErrorInfo privacy guidance only when the diff calls or assigns `Message`, `DataClassification`, `ErrorType`, or `DetailedMessage` on an expression known to be `ErrorInfo`, or when the declaration and local data flow prove that the receiving variable is `ErrorInfo`. A bare `Message(...)` call, an unrelated identifier named `Message`, or a table or field `DataClassification` edit does not satisfy this gate. For every `FeatureTelemetry.LogError`, inspect the dedicated error text and call-stack arguments in addition to explicit custom dimensions.
|
||||
|
||||
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
|
||||
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Apply the topic-specific gates above after this overlap check; in particular, bare `Message` and `DataClassification` tokens cannot admit ErrorInfo guidance. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
|
||||
|
||||
Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`.
|
||||
|
||||
|
|
|
|||
|
|
@ -38,9 +38,10 @@ Discard files that are not applicable. Retain conditionally applicable files (an
|
|||
Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against:
|
||||
|
||||
- The changed AL object names and types — especially codeunits with `Subtype = Upgrade` or `Subtype = Install`, tables and tableextensions adding or changing fields, enums and enumextensions, and objects under `Hybrid*`/`Migration`/`Upgrade` namespaces.
|
||||
- The changed triggers and procedures, weighted toward `OnCheckPreconditionsPerCompany`/`PerDatabase`, `OnUpgradePerCompany`/`PerDatabase`, `OnValidateUpgradePerCompany`/`PerDatabase`, `OnInstallAppPerCompany`/`PerDatabase`, and the `OnGetPerCompanyUpgradeTags`/`OnGetPerDatabaseUpgradeTags` subscribers.
|
||||
- The changed triggers and procedures, weighted toward `OnCheckPreconditionsPerCompany`/`PerDatabase`, `OnUpgradePerCompany`/`PerDatabase`, `OnValidateUpgradePerCompany`/`PerDatabase`, `OnInstallAppPerCompany`/`PerDatabase`, the `OnGetPerCompanyUpgradeTags`/`OnGetPerDatabaseUpgradeTags` subscribers, and helper procedures transitively reachable from those entry points.
|
||||
- Tokens extracted from the diff that relate to upgrade concerns (`Subtype = Upgrade`, `Subtype = Install`, `Upgrade Tag`, `HasUpgradeTag`, `SetUpgradeTag`, `OnCheckPreconditions`, `OnUpgrade`, `OnValidateUpgrade`, `OnInstallApp`, `DataTransfer`, `CopyFields`, `Insert`, `Modify`, `Delete`, `Rename`, `InitValue`, `ObsoleteState`, `ObsoleteReason`, `ObsoleteTag`, `DataVersion`, `ExecutionContext`, `PrimaryKey`, `key(`, `field(`, `value(`, `enum`, `enumextension`, `HybridSL`, `HybridGP`, `HybridBC`, `HybridBaseDeployment`).
|
||||
- Worklist the check-only rule when precondition or validation triggers contain writes, and the install-versus-upgrade rule when migration helpers are reachable only from an install codeunit.
|
||||
- Build the best available call graph from each precondition and validation trigger, following helper calls transitively through the surrounding unchanged source as well as changed hunks. Worklist the check-only rule when a write occurs directly in one of those triggers or in a reachable helper. Also worklist it when a PR changes the body of such a helper even if the calling trigger is unchanged. Worklist the install-versus-upgrade rule when migration helpers are reachable only from an install codeunit.
|
||||
- If repository boundaries, dynamic dispatch, missing source, or analysis limits prevent complete call-graph reachability, use the available call sites and type information but cap any reachability-dependent finding at `medium` confidence and state the incomplete reachability in the finding message.
|
||||
|
||||
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. When the diff contains no upgrade-related changes by any of the above signals, return `outcome: "not-applicable"` without evaluating files.
|
||||
|
||||
|
|
@ -58,7 +59,7 @@ For each worklist entry, evaluate the diff against the file's `## Best Practice`
|
|||
|
||||
Set `confidence` to:
|
||||
|
||||
- `high` when the detection is based on an unambiguous pattern match.
|
||||
- `high` when the detection is based on an unambiguous pattern match and any required helper reachability is fully established.
|
||||
- `medium` when detection relies on heuristics or when any frontmatter dimension was `unknown`.
|
||||
- `low` when the finding is an advisory derived only from applicability.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue