mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 17:36:53 +01:00
Regenerate microsoft/knowledge from upstream BCApps instructions
The previous LLM-generated knowledge files contained factual
hallucinations. The most visible was the claim that `FindFirst` /
`FindLast` "forces a full-table scan" on an unfiltered record - it does
not; those APIs return a single row via the current key.
Other inaccuracies the audit found and fixed:
* `FindSet(true)` was described as "taking a LockTable". The correct
upstream phrasing is that `FindSet(true)` sets
`ReadIsolation::UpdLock` on the read. UpdLock and LockTable are
related but distinct mechanisms.
* The list of production-scale tables had been invented beyond the
upstream source (e.g. "Detailed Cust. Ledg. Entry") without a
citation. The regenerated list matches the ten tables upstream lists
with their P95 row counts.
* `SetLoadFields` guidance had been augmented with an extra mechanism
claim ("the database resolves the filter using the index without
hydrating the value") not present in upstream.
Approach: full regeneration of `microsoft/knowledge/` from the six
upstream BCApps Code Review instruction files, with Microsoft Learn /
the AL language reference as a secondary source. Every claim in every
regenerated file is anchored to a verbatim upstream quote (or a Learn
URL); the audit trail lives in artifacts/trace-<domain>.json on the
session workspace.
The PR #11 transaction/error-handling cluster is preserved verbatim:
* performance/understand-implicit-transaction-boundary.md
* performance/codeunit-run-as-atomic-sub-operation.{md,good.al,bad.al}
* performance/codeunit-run-requires-prior-commit-inside-transaction.{md,good.al,bad.al}
* performance/use-tryfunction-for-error-catching-not-rollback.{md,good.al,bad.al}
* performance/avoid-commit-inside-loops.{md,good.al,bad.al}
* security/commitbehavior-attribute-scopes-explicit-commits.{md,good.al,bad.al}
* testing/transactionmodel-attribute-governs-test-transactions.{md,good.al,bad.al}
These articles already cite Microsoft Learn and were carefully
cross-referenced; the regeneration skips their topics rather than
duplicating them.
File counts after regeneration:
performance 35 .md (5 preserved + 30 new)
privacy 17 .md
security 18 .md (1 preserved + 17 new)
style 33 .md
testing 1 .md (preserved)
ui 19 .md
upgrade 18 .md
Total 141 atomic knowledge files, each strictly one rule. All pass
.github/scripts/validate_frontmatter.py with 0 errors and 0 warnings.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
parent
613c4b4019
commit
a9f3c50863
562 changed files with 6293 additions and 4869 deletions
|
|
@ -0,0 +1,7 @@
|
|||
codeunit 50227 "Sec Sample HtmlEncode Bad"
|
||||
{
|
||||
procedure BuildWelcomeHtml(UserName: Text): Text
|
||||
begin
|
||||
exit('<div>Welcome ' + UserName + '!</div>');
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,19 @@
|
|||
codeunit 50226 "Sec Sample HtmlEncode Good"
|
||||
{
|
||||
procedure BuildWelcomeHtml(UserName: Text): Text
|
||||
var
|
||||
SafeName: Text;
|
||||
begin
|
||||
SafeName := EncodeHtml(UserName);
|
||||
exit('<div>Welcome ' + SafeName + '!</div>');
|
||||
end;
|
||||
|
||||
local procedure EncodeHtml(Value: Text): Text
|
||||
begin
|
||||
Value := Value.Replace('&', '&');
|
||||
Value := Value.Replace('<', '<');
|
||||
Value := Value.Replace('>', '>');
|
||||
Value := Value.Replace('"', '"');
|
||||
exit(Value);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [html, xss, encoding, htmlencode, injection, email]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# AL has no built-in HtmlEncode — encode HTML output by hand or avoid it
|
||||
|
||||
## Description
|
||||
|
||||
AL does not ship a built-in `HtmlEncode` (or equivalent) function. Code that builds an HTML fragment — an email body, a report header, a chart label rendered as HTML — by concatenating record-field values into a string is therefore unencoded by default, and any `<`, `>`, `&`, or `"` in the user content is interpreted as markup by the receiving renderer. The result is cross-site scripting in the recipient's mail client, browser, or report viewer. The absence of a built-in encoder is non-obvious to anyone used to platforms where `HtmlEncode` is a one-liner.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Replace the four characters by hand before concatenating user content into HTML: `&` → `&` first, then `<` → `<`, `>` → `>`, `"` → `"`. Centralize the substitution in one helper so every HTML producer in the extension uses the same encoder. Better still, do not build raw HTML at all — use a structured format (JSON for an API payload, a report layout for a printed document) and let the renderer do the encoding. See sample: `al-has-no-built-in-htmlencode.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`HtmlContent := '<div>Welcome ' + UserName + '!</div>'` — any record-field value or user input concatenated directly into an HTML string. Reviewers should flag any string concatenation whose right-hand operand is a field, a parameter, or any non-literal value, and whose surrounding context contains HTML tags (`<`, `</`, `<br`, `<table`, `<a href=`). See sample: `al-has-no-built-in-htmlencode.bad.al`.
|
||||
|
|
@ -1,9 +0,0 @@
|
|||
codeunit 50215 "Sec Sample SecretCompose Bad"
|
||||
{
|
||||
procedure BuildAuthHeader(Token: Text) AuthHeader: Text
|
||||
begin
|
||||
// Token is Text, so the combined value is plaintext.
|
||||
// The whole shape should have used SecretText + SecretStrSubstNo.
|
||||
AuthHeader := StrSubstNo('Bearer %1', Token);
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,7 +0,0 @@
|
|||
codeunit 50214 "Sec Sample SecretCompose Good"
|
||||
{
|
||||
procedure BuildAuthHeader(Token: SecretText) AuthHeader: SecretText
|
||||
begin
|
||||
AuthHeader := SecretStrSubstNo('Bearer %1', Token);
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,29 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [secretstrsubstno, secrettext, composition]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Compose secrets with SecretStrSubstNo
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
SecretStrSubstNo is the SecretText analogue of StrSubstNo. The template is a regular string literal; substitution arguments may be SecretText; the return value is SecretText. Intermediate results of the composition are never materialized as plaintext.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Format SecretText templates with SecretStrSubstNo. This is the correct primitive for building authorization headers, secret URIs, and any other formatted string that embeds a SecretText. Provide the static parts of the template as a regular string literal; only the substitutions carry the secret value.
|
||||
|
||||
See sample: `compose-secrets-with-secretstrsubstno.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Using StrSubstNo (or plain string concatenation) on a plain-Text token to build an authorization header. The result is a Text containing the secret in plaintext, visible in the debugger, inspectable in snapshot debug sessions, and captured by any logging the caller does not control. SecretText should have been used end-to-end.
|
||||
|
||||
See sample: `compose-secrets-with-secretstrsubstno.bad.al`.
|
||||
|
||||
|
|
@ -1,16 +0,0 @@
|
|||
tableextension 51303 "Sec Sample VTR Bad" extends "Sales Header"
|
||||
{
|
||||
fields
|
||||
{
|
||||
// Editable user input with validation suppressed and no fallback check.
|
||||
// The user can type any string; downstream Get against Customer will fail
|
||||
// or return a wrong row.
|
||||
field(50102; "Customer No."; Code[20])
|
||||
{
|
||||
Caption = 'Customer no.';
|
||||
DataClassification = CustomerContent;
|
||||
TableRelation = Customer."No.";
|
||||
ValidateTableRelation = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1,24 +0,0 @@
|
|||
tableextension 51302 "Sec Sample VTR Good" extends "Sales Header"
|
||||
{
|
||||
fields
|
||||
{
|
||||
// User-editable field keeps ValidateTableRelation default (true).
|
||||
field(50100; "External Customer Ref"; Code[50])
|
||||
{
|
||||
Caption = 'External customer reference';
|
||||
DataClassification = CustomerContent;
|
||||
TableRelation = Customer."No.";
|
||||
}
|
||||
|
||||
// System-controlled field: validation bypass is acceptable because
|
||||
// the value is populated by controlled upstream code, not the user.
|
||||
field(50101; "System Batch Id"; Code[20])
|
||||
{
|
||||
Caption = 'System batch ID';
|
||||
DataClassification = SystemMetadata;
|
||||
TableRelation = "Job Queue Entry".ID;
|
||||
ValidateTableRelation = false;
|
||||
Editable = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1,26 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [validatetablerelation, user-input, lookup, integrity, validation]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not set ValidateTableRelation = false on fields that accept user input
|
||||
|
||||
## Description
|
||||
|
||||
`TableRelation` on a field tells the platform that the value must exist as a primary key in the related table. `ValidateTableRelation = false` suppresses that check at validation time. On system-populated fields — values the code sets from a controlled source and never displays as editable — the suppression is acceptable because the integrity guarantee comes from the upstream writer. On a field the user types into (a page field, an import column, an API payload), disabling the validation means any value at all can be written: a non-existent customer number, a typo, a deliberate bad value. The table no longer enforces the relation, and downstream code that Gets the related row with an unguarded lookup breaks.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Leave `ValidateTableRelation = true` (the default) on any field the user can set. When the default would produce unhelpful behaviour — a transient lookup that does not yet exist at validation time, a reference that uses a non-primary-key column — handle it with a targeted OnValidate trigger that performs the semantic check explicitly. Use `ValidateTableRelation = false` only when the field is genuinely system-controlled and the writer has already validated the reference.
|
||||
|
||||
See sample: `do-not-disable-validatetablerelation-on-user-input.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A `Customer No.` field on an editable page with `TableRelation = Customer."No."` and `ValidateTableRelation = false` and no OnValidate fallback. The user can type any string; the platform accepts it; a later Get against Customer fails or returns the wrong row.
|
||||
|
||||
See sample: `do-not-disable-validatetablerelation-on-user-input.bad.al`.
|
||||
|
|
@ -1,19 +0,0 @@
|
|||
codeunit 50229 "Sec Sample EventPublisher Bad"
|
||||
{
|
||||
[IntegrationEvent(false, false)]
|
||||
local procedure OnBeforeExportCustomer(CustomerNo: Code[20]; ExportCredentials: SecretText; var AllowExport: Boolean)
|
||||
begin
|
||||
end;
|
||||
|
||||
procedure ExportCustomer(CustomerNo: Code[20]; Credentials: SecretText)
|
||||
var
|
||||
AllowExport: Boolean;
|
||||
begin
|
||||
// Any subscriber on the tenant receives the credentials and
|
||||
// can flip AllowExport := true to bypass the publisher's check.
|
||||
OnBeforeExportCustomer(CustomerNo, Credentials, AllowExport);
|
||||
if not AllowExport then
|
||||
exit;
|
||||
// ... perform export
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,23 +0,0 @@
|
|||
codeunit 50228 "Sec Sample EventPublisher Good"
|
||||
{
|
||||
[IntegrationEvent(false, false)]
|
||||
local procedure OnBeforeExportCustomer(CustomerNo: Code[20])
|
||||
begin
|
||||
end;
|
||||
|
||||
procedure ExportCustomer(CustomerNo: Code[20])
|
||||
begin
|
||||
if not CallerIsAuthorizedToExport(CustomerNo) then
|
||||
Error('You are not authorized to export this customer.');
|
||||
|
||||
OnBeforeExportCustomer(CustomerNo);
|
||||
// ... perform export using credentials owned by this codeunit
|
||||
end;
|
||||
|
||||
local procedure CallerIsAuthorizedToExport(CustomerNo: Code[20]): Boolean
|
||||
begin
|
||||
// Authorization decision stays inside the publisher. Subscribers
|
||||
// receive only the customer number and cannot influence the
|
||||
// decision.
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,29 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [event, publisher, extensibility, var-parameter]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not expose sensitive data in event publishers
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
Events in AL are extensibility contracts. Every subscriber — third-party, internal, or installed after the fact — receives the full set of event parameters. Parameters that carry secrets, pre-authorization state, or variables the publisher relies on for access control effectively become public, and var-parameters can be mutated by a subscriber to alter publisher behaviour.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Design event signatures to carry only the data a subscriber legitimately needs. Do not pass SecretText, credential material, or flags the publisher depends on for access control. Guard variables such as `HasAccess`, `SkipValidation`, or `CanExport` must not be `var` parameters on an OnBefore event; notify subscribers after the internal check with value parameters they cannot mutate.
|
||||
|
||||
See sample: `do-not-expose-sensitive-data-in-event-publishers.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
An OnBeforeElevateAccess publisher that exposes `var CanAccess: Boolean` or `var SkipValidation: Boolean` — any subscriber installed on the tenant can flip it to true and bypass the check. Or a publisher that passes a SecretText parameter it obtained internally, handing it to every subscriber.
|
||||
|
||||
See sample: `do-not-expose-sensitive-data-in-event-publishers.bad.al`.
|
||||
|
||||
|
|
@ -1,14 +0,0 @@
|
|||
codeunit 51301 "Sec Sample EnvGuid Bad"
|
||||
{
|
||||
procedure GetTenantId(): Text
|
||||
begin
|
||||
// Tenant GUID hardcoded. Extension works in one environment, fails in every other.
|
||||
exit('{12345678-1234-1234-1234-123456789012}');
|
||||
end;
|
||||
|
||||
procedure GetAadApplicationId(): Text
|
||||
begin
|
||||
// AAD application GUID hardcoded. Same problem, surfaces as an authentication error.
|
||||
exit('{87654321-4321-4321-4321-210987654321}');
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,16 +0,0 @@
|
|||
codeunit 51300 "Sec Sample EnvGuid Good"
|
||||
{
|
||||
procedure KnownSystemId(): Guid
|
||||
begin
|
||||
// Stable across tenants and versions — Base Application Id.
|
||||
exit('{437dbf0e-84ff-417a-965d-ed2bb9650972}');
|
||||
end;
|
||||
|
||||
procedure GetTenantId(): Text
|
||||
var
|
||||
EnvironmentInformation: Codeunit "Environment Information";
|
||||
begin
|
||||
// Environment-specific values are retrieved at runtime.
|
||||
exit(EnvironmentInformation.GetTenantId());
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,26 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [guid, tenant-id, aad, environment, hardcoded]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Hardcoded GUIDs are only safe for well-known system identifiers
|
||||
|
||||
## Description
|
||||
|
||||
AL code sometimes carries hardcoded GUIDs. Some are platform-defined, stable across tenants and versions, and legitimately constant — the Base Application's ApplicationId (`{437dbf0e-84ff-417a-965d-ed2bb9650972}`) is the canonical example. Others identify a specific tenant, a specific Azure Active Directory application, or a specific environment; these look identical at the source-code level but are environment-bound and break the moment the extension is deployed anywhere else. Shipping an environment-specific GUID as a constant effectively locks the extension to one environment, and the failure mode in other tenants is usually an authentication error with no code-level signal pointing at the literal.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Hardcoded GUIDs are acceptable for well-known system identifiers that are stable across environments — document the identifier with a comment that names what it refers to. For tenant IDs, AAD application IDs, API subscription IDs, and any value that varies by deployment, retrieve at runtime from IsolatedStorage, configuration tables, or the platform APIs that expose the current tenant context.
|
||||
|
||||
See sample: `do-not-hardcode-environment-specific-guids.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`TenantId := '{12345678-1234-1234-1234-123456789012}';` or `AadApplicationId := '{87654321-...}';` inline in a codeunit. The extension authenticates in one environment and fails in every other; debugging starts from an AAD error message that does not mention the literal.
|
||||
|
||||
See sample: `do-not-hardcode-environment-specific-guids.bad.al`.
|
||||
|
|
@ -1,6 +0,0 @@
|
|||
permissionset 50201 "Sec Sample Full Access"
|
||||
{
|
||||
Assignable = true;
|
||||
Caption = 'Full Access (sample anti-pattern)';
|
||||
Permissions = tabledata * = RIMD;
|
||||
}
|
||||
|
|
@ -1,9 +0,0 @@
|
|||
permissionset 50200 "Sec Sample Sales Order Entry"
|
||||
{
|
||||
Assignable = true;
|
||||
Caption = 'Sales Order Entry (sample)';
|
||||
Permissions =
|
||||
tabledata "Sales Header" = RIM,
|
||||
tabledata "Sales Line" = RIMD,
|
||||
tabledata Customer = R;
|
||||
}
|
||||
|
|
@ -1,29 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [permissionset, least-privilege, rimd, tabledata]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Follow least privilege in permission sets
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
Permission sets define the tabledata and object rights granted to every user or role assigned to them. A permission set that grants RIMD on tabledata * hands every caller full control over every table the extension exposes, which is never the shape of access any real role requires. Over-broad permission sets are a persistent source of privilege-escalation risk: once assigned, they are rarely audited.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Enumerate the specific tabledata objects a role needs and grant only the letters (R, I, M, D) that role genuinely uses. A sales order-entry role typically needs RIM on Sales Header, RIMD on Sales Line, and R on Customer — not blanket RIMD. Permission sets SHOULD be granular and role-shaped; a single permission set that covers every role in an extension is a design smell.
|
||||
|
||||
See sample: `follow-least-privilege-in-permission-sets.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Granting `tabledata * = RIMD` (or any wildcard with I, M, or D) in a permission set. This bypasses any meaningful separation of duties the extension could enforce and gives unreviewed code paths the ability to insert, modify, and delete on any table.
|
||||
|
||||
See sample: `follow-least-privilege-in-permission-sets.bad.al`.
|
||||
|
||||
|
|
@ -0,0 +1,10 @@
|
|||
codeunit 50234 "Sec Sample LastErrText"
|
||||
{
|
||||
procedure RunWithCapture(var ErrorLog: Record "Integration Log")
|
||||
begin
|
||||
if not Codeunit.Run(Codeunit::"My Worker") then begin
|
||||
ErrorLog."Error Text" := CopyStr(GetLastErrorText(), 1, MaxStrLen(ErrorLog."Error Text"));
|
||||
ErrorLog.Insert(true);
|
||||
end;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [getlasterrortext, error-text, classification, privacy, review-scope]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Storing GetLastErrorText() in table fields is a privacy finding, not a security finding
|
||||
|
||||
## Description
|
||||
|
||||
It is tempting to flag any code that calls `GetLastErrorText()` and writes the result into a table field (or displays it to end users) as a security issue, on the assumption that the error text might leak credentials or system internals. In Business Central, that pattern is treated as a **privacy** concern instead: AL `Error` text frequently contains customer content (record keys, field values, document numbers) rather than infrastructure details, and the appropriate review owner is the privacy/DataClassification reviewer. A security reviewer should not raise a finding for `GetLastErrorText()` storage on the grounds that it might expose secrets; that risk is covered elsewhere by the rules that prevent secrets from appearing in error messages in the first place (see `secrettext-for-credentials.md`).
|
||||
|
||||
## Best Practice
|
||||
|
||||
When auditing AL changes for security, ignore patterns where `GetLastErrorText()` is captured into a table or shown to users — leave those to the privacy review. Security findings on error text should be limited to the construction of the `Error()` call itself: secrets, paths, or technical internals being interpolated into the error before it is raised. See sample: `getlasterrortext-storage-is-privacy-not-security.bad.al` for the pattern that is *not* a security finding.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Filing a security finding such as "GetLastErrorText() stored in field — potential information disclosure" against AL code that captures an error for later inspection. The finding is in the wrong domain and crowds out the actual security signal. The mirror anti-pattern is silencing genuine `Error('... %1 ...', SecretValue)` constructions on the grounds that "error text is privacy" — those *are* security findings because they create the leak, regardless of where the text ends up afterwards.
|
||||
|
|
@ -0,0 +1,4 @@
|
|||
permissionset 50204 "Sec Sample Report Runner Bad"
|
||||
{
|
||||
Permissions = tabledata "G/L Entry" = RIMD;
|
||||
}
|
||||
|
|
@ -0,0 +1,4 @@
|
|||
permissionset 50203 "Sec Sample Report Runner"
|
||||
{
|
||||
Permissions = tabledata "G/L Entry" = ri;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [permissionset, indirect-permissions, ri, ii, mi, di, code-mediated]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use indirect permissions when access must be code-mediated
|
||||
|
||||
## Description
|
||||
|
||||
In a `permissionset`, uppercase letters (`R`, `I`, `M`, `D`) grant **direct** permissions: the assignee can read, insert, modify, or delete the table data through any UI or API surface. Lowercase letters (`r`, `i`, `m`, `d`) grant **indirect** permissions: the operation is allowed only when it is invoked from AL code that itself holds the corresponding direct permission. Indirect permissions let a role consume privileged tables through controlled procedures (a report, a posting routine) without giving users a way to read or change those tables outside the intended code path.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Use indirect permissions (`ri`, `ii`, `mi`, `di`) when a role needs access to a sensitive table only through a specific codeunit or report — for example, a "Report Runner" role that reads `G/L Entry` only via published reports. Pair the indirect grant with the codeunit or report that mediates access; that object's own permissions (or InherentPermissions) supply the direct rights. Document why indirect permissions are required in the permission set or in the consuming object's comments. See sample: `indirect-permissions-for-elevated-access.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Granting `RIMD` on a sensitive table when the role only needs to view it through a report — for example `tabledata "G/L Entry" = RIMD` on a "Report Runner" role. Users assigned that role can now query and modify ledger entries directly through any client that respects the permission, bypassing the report entirely. Reviewers should look for uppercase grants on system-of-record tables (G/L Entry, ledger entries, posted documents) where the consuming code path is clearly read-through-report or read-through-API. See sample: `indirect-permissions-for-elevated-access.bad.al`.
|
||||
|
|
@ -0,0 +1,20 @@
|
|||
codeunit 50206 "Sec Sample Inherent Bad"
|
||||
{
|
||||
[InherentPermissions(PermissionObjectType::TableData, Database::"Sales Header", 'RIMD')]
|
||||
[InherentEntitlements(Entitlement::"Dynamics 365 Business Central Premium")]
|
||||
procedure GetCustomerName(CustomerNo: Code[20]): Text
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
if Customer.Get(CustomerNo) then
|
||||
exit(Customer.Name);
|
||||
end;
|
||||
|
||||
[InherentEntitlements(Entitlement::"Dynamics 365 Business Central Premium")]
|
||||
procedure CheckItemExists(ItemNo: Code[20]): Boolean
|
||||
var
|
||||
Item: Record Item;
|
||||
begin
|
||||
exit(Item.Get(ItemNo));
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,19 @@
|
|||
codeunit 50205 "Sec Sample Inherent Good"
|
||||
{
|
||||
[InherentPermissions(PermissionObjectType::TableData, Database::Customer, 'r')]
|
||||
procedure GetCustomerName(CustomerNo: Code[20]): Text
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
if Customer.Get(CustomerNo) then
|
||||
exit(Customer.Name);
|
||||
end;
|
||||
|
||||
[InherentPermissions(PermissionObjectType::TableData, Database::Item, 'r')]
|
||||
procedure CheckItemExists(ItemNo: Code[20]): Boolean
|
||||
var
|
||||
Item: Record Item;
|
||||
begin
|
||||
exit(Item.Get(ItemNo));
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [inherentpermissions, inherententitlements, attribute, least-privilege, procedure]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Grant the minimum InherentPermissions a procedure needs
|
||||
|
||||
## Description
|
||||
|
||||
`[InherentPermissions(PermissionObjectType::..., ...)]` and `[InherentEntitlements(Entitlement::...)]` are method-level attributes that let a procedure perform an operation on the listed object even when the caller's permission set does not allow it. They effectively elevate the caller for the duration of the procedure. The grant therefore needs to be as narrow as the procedure's actual work — both in object scope (the specific table) and in operation (`'r'` versus `'RIMD'`). Overly broad inherent permissions silently expand the attack surface of every codeunit that calls the procedure.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Match the inherent permission to the procedure's body: a procedure that only reads `Customer.Name` declares `[InherentPermissions(PermissionObjectType::TableData, Database::Customer, 'r')]`, not `'RIMD'`. Pick the inherent entitlement that matches the lowest tier the procedure should run under — do not require Premium for a procedure that performs an Essential-tier check. See sample: `inherent-permissions-minimal-grant.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Declaring `[InherentPermissions(..., 'RIMD')]` on a read-only procedure (`GetCustomerName`), or `[InherentEntitlements(Entitlement::"Dynamics 365 Business Central Premium")]` on a procedure that performs a simple existence check. Reviewers should compare the attribute's permission letters against what the procedure body actually does and flag any grant broader than the operations performed. See sample: `inherent-permissions-minimal-grant.bad.al`.
|
||||
|
|
@ -0,0 +1,7 @@
|
|||
codeunit 50229 "Sec Sample EventSecret Bad"
|
||||
{
|
||||
[IntegrationEvent(false, false)]
|
||||
local procedure OnBeforeSendRequest(var ApiKey: Text; var Password: Text; var RequestUrl: Text)
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,7 @@
|
|||
codeunit 50228 "Sec Sample EventSecret Good"
|
||||
{
|
||||
[IntegrationEvent(false, false)]
|
||||
local procedure OnBeforeSendRequest(var RequestPayload: JsonObject; var IsHandled: Boolean)
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [integrationevent, eventsubscriber, secrets, credentials, publisher]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not pass credentials or secrets through IntegrationEvent parameters
|
||||
|
||||
## Description
|
||||
|
||||
`[IntegrationEvent]` publishes a hook that any extension can subscribe to. Every parameter of the event signature is visible to every subscriber — including `var` parameters, which subscribers can both read and modify. A publisher that includes an API key, password, bearer token, or other secret in the event signature hands that secret to every subscriber on the tenant, including subscribers in extensions the publisher has no relationship with. There is no permission or partner-only filter that limits who may subscribe.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Restrict event payloads to the non-sensitive context a subscriber legitimately needs: the business record being processed (a `Customer`), the operation being performed, an `IsHandled` flag that lets a subscriber skip the default behaviour, and a mutable payload object whose contents the publisher controls. Authentication is handled by the publisher before or after the event, never inside the parameters. See sample: `integrationevent-must-not-expose-secrets.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`[IntegrationEvent(false, false)] procedure OnBeforeSendRequest(var ApiKey: Text; var Password: Text; var RequestUrl: Text)` — any extension on the tenant can subscribe, read `ApiKey` and `Password`, and persist them elsewhere. Reviewers should flag any event parameter whose name or type suggests a secret (`ApiKey`, `Token`, `Password`, `Secret`, `Credential`, `SecretText` — even `SecretText` should not flow through an event surface). See sample: `integrationevent-must-not-expose-secrets.bad.al`.
|
||||
|
|
@ -0,0 +1,19 @@
|
|||
codeunit 50231 "Sec Sample EventGuard Bad"
|
||||
{
|
||||
procedure CheckPermissionsForTable(TableNo: Integer)
|
||||
var
|
||||
HasAccess: Boolean;
|
||||
SkipValidation: Boolean;
|
||||
begin
|
||||
OnBeforeCheckPermissions(HasAccess, SkipValidation, TableNo);
|
||||
if SkipValidation then
|
||||
exit;
|
||||
if not HasAccess then
|
||||
Error('Access denied.');
|
||||
end;
|
||||
|
||||
[IntegrationEvent(false, false)]
|
||||
local procedure OnBeforeCheckPermissions(var HasAccess: Boolean; var SkipValidation: Boolean; TableNo: Integer)
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
codeunit 50230 "Sec Sample EventGuard Good"
|
||||
{
|
||||
procedure CheckPermissionsForTable(TableNo: Integer)
|
||||
var
|
||||
HasAccess: Boolean;
|
||||
begin
|
||||
HasAccess := PerformInternalCheck(TableNo);
|
||||
if not HasAccess then
|
||||
Error('Access denied.');
|
||||
OnAfterCheckPermissions(TableNo, HasAccess);
|
||||
end;
|
||||
|
||||
local procedure PerformInternalCheck(TableNo: Integer): Boolean
|
||||
begin
|
||||
exit(true);
|
||||
end;
|
||||
|
||||
[IntegrationEvent(false, false)]
|
||||
local procedure OnAfterCheckPermissions(TableNo: Integer; HasAccess: Boolean)
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [integrationevent, var, guard, ishandled, bypass, security-check]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not expose security guards as `var` parameters on IntegrationEvent
|
||||
|
||||
## Description
|
||||
|
||||
A `var` parameter on an `[IntegrationEvent]` is a mutable hook: any subscriber can overwrite the value and the publisher will see the new value when control returns. That is the right shape for "let an extension contribute to a payload"; it is the wrong shape for "let an extension confirm a security decision". A `var HasAccess: Boolean` or `var SkipValidation: Boolean` lets any subscriber on the tenant flip the result of the publisher's permission check to `true` (or set "skip" to `true`) before the publisher reads it. The publisher's check becomes advisory, which is the same as not having a check.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Keep the security decision inside the publisher, where it is not bypassable. Fire an `OnAfter*` informational event after the check completes, with the result passed by value (not `var`) so subscribers can react — log, audit, surface a warning — but cannot rewrite the outcome. When subscribers legitimately need to add their own checks, expose an `OnAfterCheckPermissions(...)` that can only tighten access (e.g., a subscriber can `Error()`), never loosen it. See sample: `integrationevent-var-parameter-bypasses-security-guards.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`OnBeforeCheckPermissions(var HasAccess: Boolean; var SkipValidation: Boolean; TableNo: Integer)`, followed in the caller by `if SkipValidation then exit;`. Any subscriber sets `SkipValidation := true` and the check is gone. Reviewers should flag any `IntegrationEvent` whose signature contains a `var Boolean` whose name reads like a security decision (`HasAccess`, `IsAllowed`, `SkipValidation`, `BypassCheck`, `IsAuthorized`). See sample: `integrationevent-var-parameter-bypasses-security-guards.bad.al`.
|
||||
|
|
@ -0,0 +1,16 @@
|
|||
codeunit 50216 "Sec Sample IsoStorage Bad"
|
||||
{
|
||||
procedure GetApiKey(): Text
|
||||
var
|
||||
ApiKey: Text;
|
||||
begin
|
||||
if IsolatedStorage.Contains('ApiKey', DataScope::Module) then
|
||||
IsolatedStorage.Get('ApiKey', DataScope::Module, ApiKey);
|
||||
exit(ApiKey);
|
||||
end;
|
||||
|
||||
procedure SetApiKey(NewKey: Text)
|
||||
begin
|
||||
IsolatedStorage.SetEncrypted('ApiKey', NewKey, DataScope::Module);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,15 @@
|
|||
codeunit 50215 "Sec Sample IsoStorage Good"
|
||||
{
|
||||
local procedure GetApiKey(var ApiKey: SecretText): Boolean
|
||||
begin
|
||||
if not IsolatedStorage.Contains('ApiKey', DataScope::Module) then
|
||||
exit(false);
|
||||
IsolatedStorage.Get('ApiKey', DataScope::Module, ApiKey);
|
||||
exit(true);
|
||||
end;
|
||||
|
||||
internal procedure SetApiKey(NewKey: Text)
|
||||
begin
|
||||
IsolatedStorage.SetEncrypted('ApiKey', NewKey, DataScope::Module);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [isolatedstorage, local, internal, public, getter, setter, encapsulation]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Procedures that read or write IsolatedStorage must not be public
|
||||
|
||||
## Description
|
||||
|
||||
`IsolatedStorage` partitions its data by extension: values written by one extension are unreadable to another. That guarantee assumes the owning extension does not voluntarily expose its storage through a public API. A `public` procedure on a codeunit that calls `IsolatedStorage.Get`, `IsolatedStorage.Set`, `IsolatedStorage.SetEncrypted`, `IsolatedStorage.Contains`, or `IsolatedStorage.Delete` defeats the isolation: any other extension on the same tenant can call that procedure and obtain (or overwrite) the secret. The platform's per-extension boundary becomes a per-procedure boundary, and there is no per-procedure boundary.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Mark every procedure that touches `IsolatedStorage` as `local` (visible only inside its containing object) or `internal` (visible only inside the owning extension). Provide consumers with a narrow, intent-specific API — for example, "send notification to configured webhook" rather than "give me the webhook secret." See sample: `isolatedstorage-access-must-be-local-or-internal.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A public `GetApiKey()` returning the stored value, or a public `SetApiKey(NewKey: Text)` that calls `IsolatedStorage.SetEncrypted`. Both turn the extension into a confused deputy that hands out (or accepts overwrites of) its own secrets on behalf of any caller on the tenant. Reviewers should flag any procedure whose body references `IsolatedStorage` and whose declaration omits `local` or `internal`. See sample: `isolatedstorage-access-must-be-local-or-internal.bad.al`.
|
||||
|
|
@ -0,0 +1,15 @@
|
|||
codeunit 50220 "Sec Sample DataScope Bad"
|
||||
{
|
||||
internal procedure StoreCompanyWebhook(WebhookUrl: Text)
|
||||
begin
|
||||
IsolatedStorage.SetEncrypted('WebhookUrl', WebhookUrl, DataScope::Module);
|
||||
end;
|
||||
|
||||
local procedure ReadCompanyWebhook(var WebhookUrl: SecretText): Boolean
|
||||
begin
|
||||
if not IsolatedStorage.Contains('WebhookUrl', DataScope::Company) then
|
||||
exit(false);
|
||||
IsolatedStorage.Get('WebhookUrl', DataScope::Company, WebhookUrl);
|
||||
exit(true);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,20 @@
|
|||
codeunit 50219 "Sec Sample DataScope Good"
|
||||
{
|
||||
internal procedure StoreTenantApiKey(ApiKey: Text)
|
||||
begin
|
||||
IsolatedStorage.SetEncrypted('TenantApiKey', ApiKey, DataScope::Module);
|
||||
end;
|
||||
|
||||
internal procedure StoreCompanyWebhook(WebhookUrl: Text)
|
||||
begin
|
||||
IsolatedStorage.SetEncrypted('WebhookUrl', WebhookUrl, DataScope::Company);
|
||||
end;
|
||||
|
||||
local procedure ReadCompanyWebhook(var WebhookUrl: SecretText): Boolean
|
||||
begin
|
||||
if not IsolatedStorage.Contains('WebhookUrl', DataScope::Company) then
|
||||
exit(false);
|
||||
IsolatedStorage.Get('WebhookUrl', DataScope::Company, WebhookUrl);
|
||||
exit(true);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [isolatedstorage, datascope, module, company, user, scope]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Pick the right IsolatedStorage DataScope for the secret's lifetime
|
||||
|
||||
## Description
|
||||
|
||||
`IsolatedStorage` read and write methods take a `DataScope` parameter that decides which slice of storage the value belongs to. The choice is not a stylistic one — it changes which callers, in which company and under which user, can read the value back. Two scopes cover the common cases for app-level secrets: `DataScope::Module` stores the value once for the whole extension, isolated to that extension on the tenant — the right scope for app-specific secrets such as a global API key or service account. `DataScope::Company` stores the value per company, so each company on the tenant has its own slot — the right scope for company-specific secrets such as a per-company webhook URL or a per-company integration token. A per-user scope also exists for values that belong to an individual user.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Choose `Module` when the secret is the same for every company and every user under the extension (a single tenant-wide API key). Choose `Company` when each company has its own integration credentials. Choose the user scope only when the secret is genuinely per-user. Use the same `DataScope` value on `Set`/`SetEncrypted`, `Get`, `Contains`, and `Delete` for the same key — mixing scopes for the same logical secret produces silent "not found" results. See sample: `isolatedstorage-datascope-module-vs-company.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Defaulting every call to `DataScope::Module` regardless of intent — storing a per-company webhook URL under `Module` means every company on the tenant shares the same URL. Or the inverse: storing a tenant-wide API key under `Company` means each company-switch effectively loses the key. Reviewers should look for cross-method inconsistency (`Set` under `Module`, `Get` under `Company`) and for scope choices that contradict the value's documented lifetime. See sample: `isolatedstorage-datascope-module-vs-company.bad.al`.
|
||||
|
|
@ -0,0 +1,7 @@
|
|||
codeunit 50218 "Sec Sample SetEncrypted Bad"
|
||||
{
|
||||
internal procedure StoreApiKey(ApiKeyValue: Text)
|
||||
begin
|
||||
IsolatedStorage.Set('ApiKey', ApiKeyValue, DataScope::Module);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,17 @@
|
|||
codeunit 50217 "Sec Sample SetEncrypted Good"
|
||||
{
|
||||
internal procedure StoreApiKey(ApiKeyValue: Text)
|
||||
begin
|
||||
if StrLen(ApiKeyValue) > 200 then
|
||||
Error('API key too long for encrypted storage');
|
||||
IsolatedStorage.SetEncrypted('ApiKey', ApiKeyValue, DataScope::Module);
|
||||
end;
|
||||
|
||||
local procedure ReadApiKey(var ApiKey: SecretText): Boolean
|
||||
begin
|
||||
if not IsolatedStorage.Contains('ApiKey', DataScope::Module) then
|
||||
exit(false);
|
||||
IsolatedStorage.Get('ApiKey', DataScope::Module, ApiKey);
|
||||
exit(true);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [isolatedstorage, setencrypted, encryption, secret, storage]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Prefer IsolatedStorage.SetEncrypted over Set for sensitive values
|
||||
|
||||
## Description
|
||||
|
||||
`IsolatedStorage` exposes two write entry points: `Set` stores the value as-is, and `SetEncrypted` stores it encrypted at rest. Both are scoped per extension, but only `SetEncrypted` adds the additional protection that the value is not readable from the underlying storage by anything that bypasses the AL `IsolatedStorage` API. The choice between them is by intent: configuration that is not sensitive (a user preference, a default flag) can use `Set`; anything that would harm the tenant if leaked — API keys, tokens, connection strings, OAuth client secrets — uses `SetEncrypted`.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Use `IsolatedStorage.SetEncrypted` for every value that meets the definition of a secret. Pair it with the matching retrieval pattern: `IsolatedStorage.Contains` to test for presence and `IsolatedStorage.Get` (preferably with a `SecretText` destination) to read. Constrain the input length before storing — long values can exceed the encrypted-storage size limit and the write will fail at runtime. See sample: `isolatedstorage-setencrypted-for-sensitive-values.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`IsolatedStorage.Set('ApiKey', ApiKeyValue, DataScope::Module)` — the key is now sitting in storage unencrypted, and any future incident that exposes the underlying storage exposes the key. Reviewers should flag any `IsolatedStorage.Set` whose key name or surrounding context suggests a secret (`ApiKey`, `Token`, `Password`, `Secret`, `ClientSecret`). See sample: `isolatedstorage-setencrypted-for-sensitive-values.bad.al`.
|
||||
|
|
@ -1,12 +0,0 @@
|
|||
codeunit 50242 "Sec Sample RecordRef Good"
|
||||
{
|
||||
internal procedure ArchiveRecord(RecId: RecordId)
|
||||
var
|
||||
RecRef: RecordRef;
|
||||
begin
|
||||
RecRef.Open(RecId.TableNo);
|
||||
RecRef.Get(RecId);
|
||||
RecRef.Delete();
|
||||
RecRef.Close();
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,26 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [recordref, recordid, table-no, scope, inherentpermissions]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Keep caller-driven RecordRef.Open procedures non-public
|
||||
|
||||
## Description
|
||||
|
||||
A codeunit can hold permissions or `InherentPermissions` that its callers do not have. If it exposes a public procedure that accepts a table number or RecordId and calls `RecordRef.Open`, another extension can call that procedure to make the privileged codeunit open tables on its behalf. That turns a generic helper into a permission-bypass surface, especially for system tables.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Procedures that call `RecordRef.Open` with a caller-provided table number must be `local`, `internal`, or `[Scope('OnPrem')]`. If the procedure truly must be public in SaaS, validate the table number against a narrow allowlist before opening the RecordRef.
|
||||
|
||||
See sample: `keep-recordref-open-callers-non-public.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A public helper such as `ArchiveRecord(RecId: RecordId)` that opens `RecId.TableNo` and then reads, modifies, or deletes through RecordRef. The helper compiles, but it lets untrusted callers choose which table the privileged code opens.
|
||||
|
||||
See sample: `keep-recordref-open-callers-non-public.bad.al`.
|
||||
|
|
@ -1,10 +0,0 @@
|
|||
codeunit 50207 "Sec Sample HardcodedSecret Bad"
|
||||
{
|
||||
var
|
||||
HardcodedApiKeyLbl: Label 'sk-live-1234567890abcdef', Locked = true;
|
||||
|
||||
procedure GetApiKey(): Text
|
||||
begin
|
||||
exit(HardcodedApiKeyLbl);
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,12 +0,0 @@
|
|||
codeunit 50206 "Sec Sample HardcodedSecret Good"
|
||||
{
|
||||
procedure GetApiKey() ApiKey: SecretText
|
||||
var
|
||||
StoredValue: SecretText;
|
||||
begin
|
||||
if IsolatedStorage.Contains('ApiKey', DataScope::Module) then
|
||||
if IsolatedStorage.Get('ApiKey', DataScope::Module, StoredValue) then
|
||||
exit(StoredValue);
|
||||
Error('API key is not configured.');
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,29 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [secrets, credentials, hardcoded, label, apikey]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Never hardcode secrets in AL
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
A secret embedded in AL source — API key, password, connection string, token — lives forever: in the app package, in source control history, in every debugger session that sees the assignment, and in any log that captures the containing variable. Rotation is effectively impossible without a new release, and the blast radius covers every tenant the extension is installed in.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Retrieve secrets at runtime from a protected store: Azure Key Vault for production workloads (see prefer-azure-key-vault-for-production-secrets) or IsolatedStorage for tenant-local encrypted values (see use-isolated-storage-for-module-and-company-secrets). Carry the retrieved value in a SecretText variable end-to-end (see use-secrettext-for-credentials).
|
||||
|
||||
See sample: `never-hardcode-secrets-in-al.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Assigning a secret literal to a Text, Code, or Label variable (including labels marked as constants). The secret is now part of the compiled app and indistinguishable from non-sensitive content to callers and tools.
|
||||
|
||||
See sample: `never-hardcode-secrets-in-al.bad.al`.
|
||||
|
||||
|
|
@ -0,0 +1,19 @@
|
|||
codeunit 50214 "Sec Sample NonDebug Bad"
|
||||
{
|
||||
procedure BuildConnectionString(ApiKey: SecretText): Text
|
||||
begin
|
||||
exit('Server=db.example.com;Key=' + ApiKey.Unwrap());
|
||||
end;
|
||||
|
||||
procedure ParseSessionToken(Response: HttpResponseMessage; var SessionToken: SecretText)
|
||||
var
|
||||
ResponseText: Text;
|
||||
JsonObject: JsonObject;
|
||||
JsonToken: JsonToken;
|
||||
begin
|
||||
Response.Content.ReadAs(ResponseText);
|
||||
JsonObject.ReadFrom(ResponseText);
|
||||
JsonObject.Get('access_token', JsonToken);
|
||||
SessionToken := JsonToken.AsValue().AsText();
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,21 @@
|
|||
codeunit 50213 "Sec Sample NonDebug Good"
|
||||
{
|
||||
[NonDebuggable]
|
||||
procedure BuildConnectionString(ApiKey: SecretText): Text
|
||||
begin
|
||||
exit('Server=db.example.com;Key=' + ApiKey.Unwrap());
|
||||
end;
|
||||
|
||||
[NonDebuggable]
|
||||
procedure ParseSessionToken(Response: HttpResponseMessage; var SessionToken: SecretText)
|
||||
var
|
||||
ResponseText: Text;
|
||||
JsonObject: JsonObject;
|
||||
JsonToken: JsonToken;
|
||||
begin
|
||||
Response.Content.ReadAs(ResponseText);
|
||||
JsonObject.ReadFrom(ResponseText);
|
||||
JsonObject.Get('access_token', JsonToken);
|
||||
SessionToken := JsonToken.AsValue().AsText();
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [nondebuggable, attribute, secrettext, unwrap, debugger]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Mark procedures that call SecretText.Unwrap() as [NonDebuggable]
|
||||
|
||||
## Description
|
||||
|
||||
`SecretText` transit — assignment, parameter passing, and return values — is auto-protected: the debugger sees a redacted placeholder, not the value. The protection ends the moment code calls `.Unwrap()`, which converts the `SecretText` back to plain `Text`. From that point on, the local variable holding the result is visible in the debugger like any other `Text`. The `[NonDebuggable]` attribute marks a procedure so that none of its locals or parameters are visible to the debugger during execution, which is exactly what is needed for any procedure that performs an `Unwrap()` or that otherwise materializes a secret as `Text` (for example, while parsing a JSON response to extract an access token).
|
||||
|
||||
## Best Practice
|
||||
|
||||
Apply `[NonDebuggable]` to any procedure whose body calls `.Unwrap()` on a `SecretText`, and to any procedure that constructs a `SecretText` from a `Text` source (such as a procedure that reads a JSON response body and converts the resulting `Text` into a `SecretText` for the caller). Keep the unwrap window as small as possible — ideally a single one-line helper that hands the unwrapped value straight to the consuming API. See sample: `nondebuggable-required-when-unwrapping-secrettext.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling `ApiKey.Unwrap()` inside a procedure that is not marked `[NonDebuggable]`. The unwrapped value is now an ordinary `Text` local and the debugger will display it, defeating the purpose of using `SecretText` in the first place. Reviewers should flag any `Unwrap()` call in a procedure that lacks the attribute, and any procedure that parses a credential out of a response (`access_token`, `id_token`, `client_secret`) without the attribute. See sample: `nondebuggable-required-when-unwrapping-secrettext.bad.al`.
|
||||
|
|
@ -0,0 +1,10 @@
|
|||
permissionset 50201 "Sec Sample Full Access"
|
||||
{
|
||||
Permissions = tabledata * = RIMD;
|
||||
}
|
||||
|
||||
permissionset 50202 "Sec Sample Basic User"
|
||||
{
|
||||
Permissions = table * = X,
|
||||
tabledata * = R;
|
||||
}
|
||||
|
|
@ -0,0 +1,8 @@
|
|||
permissionset 50200 "Sec Sample Sales Entry"
|
||||
{
|
||||
Permissions = tabledata "Sales Header" = RIM,
|
||||
tabledata "Sales Line" = RIMD,
|
||||
tabledata Customer = R,
|
||||
table "Sales Header" = X,
|
||||
table "Sales Line" = X;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [permissionset, wildcard, rimd, tabledata, least-privilege]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Avoid wildcard grants in permission sets
|
||||
|
||||
## Description
|
||||
|
||||
A `permissionset` object can grant access object-by-object or with the `*` wildcard. Wildcard grants — `tabledata * = RIMD` (Read/Insert/Modify/Delete on every table) and `table * = X` (Execute on every table object) — collapse the principle of least privilege into a single line and are almost never what the author intended. The grant binds for the lifetime of the permission set wherever it is assigned, including indirectly via role assignment. Permission sets should be granular and role-specific, enumerating only the objects the role actually needs.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Enumerate each `tabledata` and each `table` entry explicitly. Grant only the letters required: `R` for read-only consumers, `RIM` for editors that do not delete, `RIMD` only for owners of the data. When a role needs Execute on objects, list those objects rather than using `table *`. See sample: `permission-set-avoid-wildcard-grants.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`Permissions = tabledata * = RIMD;` and `Permissions = table * = X, tabledata * = R;` — both grant access to objects the role's author never inspected, and the grant silently broadens every time a new table ships in the platform or in another extension. Reviewers should flag any `*` on the left-hand side of a `tabledata` or `table` entry. See sample: `permission-set-avoid-wildcard-grants.bad.al`.
|
||||
|
|
@ -1,25 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [keyvault, azure, secrets, rotation, audit]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Prefer Azure Key Vault for production secrets
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
Azure Key Vault is an external secret store that supports central management, rotation, and access auditing. The Business Central system application exposes integration APIs that retrieve Key Vault secrets at runtime. IsolatedStorage, by contrast, is a per-tenant local encrypted store with no central rotation or audit story.
|
||||
|
||||
## Best Practice
|
||||
|
||||
For production workloads that require secret rotation, access auditing, and separation between secret custodians and app developers, Azure Key Vault SHOULD be the store of record. Retrieve secrets into a SecretText variable on demand, cache only as long as the call requires, and never persist the retrieved plaintext anywhere the extension does not control. IsolatedStorage MAY be used when a per-tenant local encrypted store is all that is required.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Treating IsolatedStorage as the long-term home for secrets in a multi-tenant production extension where secret rotation, central revocation, or access auditing are required.
|
||||
|
||||
|
|
@ -1,4 +1,4 @@
|
|||
codeunit 50243 "Sec Sample RecordRef Bad"
|
||||
codeunit 50233 "Sec Sample RecRef Bad"
|
||||
{
|
||||
procedure ArchiveRecord(RecId: RecordId)
|
||||
var
|
||||
|
|
@ -0,0 +1,29 @@
|
|||
codeunit 50232 "Sec Sample RecRef Good"
|
||||
{
|
||||
internal procedure ArchiveRecord(RecId: RecordId)
|
||||
var
|
||||
RecRef: RecordRef;
|
||||
begin
|
||||
RecRef.Open(RecId.TableNo);
|
||||
RecRef.Get(RecId);
|
||||
RecRef.Delete();
|
||||
RecRef.Close();
|
||||
end;
|
||||
|
||||
procedure ArchiveAllowedRecord(RecId: RecordId)
|
||||
var
|
||||
RecRef: RecordRef;
|
||||
begin
|
||||
if not IsAllowedTable(RecId.TableNo) then
|
||||
Error('Operation not permitted on this table.');
|
||||
RecRef.Open(RecId.TableNo);
|
||||
RecRef.Get(RecId);
|
||||
RecRef.Delete();
|
||||
RecRef.Close();
|
||||
end;
|
||||
|
||||
local procedure IsAllowedTable(TableNo: Integer): Boolean
|
||||
begin
|
||||
exit(TableNo in [Database::Customer, Database::Vendor]);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [recordref, open, public, system-table, scope-onprem, confused-deputy, saas]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Procedures that RecordRef.Open a caller-provided table must not be public
|
||||
|
||||
## Description
|
||||
|
||||
When a codeunit holds permission to system tables — directly, via a permission set granted at install, or via `[InherentPermissions]` — and exposes a public procedure that accepts a table number (or a `RecordId`, from which the table number is derived) and calls `RecordRef.Open` on it, the procedure becomes a confused deputy. Any other extension on the same tenant can invoke the procedure with the table number of a system table the calling extension does not own permissions for and obtain access to its rows through the wrapper. This is especially acute in SaaS: an on-premises-style extension that holds broad permissions can be exploited by a co-tenant extension that calls its public surface.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Mark such procedures `local` (callable only inside the containing object), `internal` (callable only inside the owning extension), or `[Scope('OnPrem')]` (not callable from SaaS extensions). If the procedure must be public, validate the table number against an allow-list before `RecordRef.Open` — `if not IsAllowedTable(RecId.TableNo) then Error(...)` — so the caller cannot specify an arbitrary table. See sample: `recordref-open-with-caller-table-must-not-be-public.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`procedure ArchiveRecord(RecId: RecordId)` (public by default) whose body calls `RecRef.Open(RecId.TableNo)` and then reads, modifies, or deletes the record. Reviewers should flag any procedure that is public (no `local`/`internal`/`[Scope('OnPrem')]`), takes a `RecordId`, `Integer` table number, or `Variant` as a parameter, and calls `RecordRef.Open` with that parameter — unless an allow-list check on the table number precedes the open. See sample: `recordref-open-with-caller-table-must-not-be-public.bad.al`.
|
||||
|
|
@ -0,0 +1,12 @@
|
|||
codeunit 50212 "Sec Sample SecretSubst Bad"
|
||||
{
|
||||
procedure BuildAuthHeader(Token: SecretText): Text
|
||||
begin
|
||||
exit(StrSubstNo('Bearer %1', Token.Unwrap()));
|
||||
end;
|
||||
|
||||
procedure BuildSecretUri(BaseUrl: Text; ApiKey: SecretText): Text
|
||||
begin
|
||||
exit(BaseUrl + '?key=' + ApiKey.Unwrap());
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,12 @@
|
|||
codeunit 50211 "Sec Sample SecretSubst Good"
|
||||
{
|
||||
procedure BuildAuthHeader(Token: SecretText): SecretText
|
||||
begin
|
||||
exit(SecretStrSubstNo('Bearer %1', Token));
|
||||
end;
|
||||
|
||||
procedure BuildSecretUri(BaseUrl: Text; ApiKey: SecretText): SecretText
|
||||
begin
|
||||
exit(SecretStrSubstNo('%1?key=%2', BaseUrl, ApiKey));
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [secretstrsubstno, secrettext, strsubstno, format, compose]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use SecretStrSubstNo to compose strings that contain secrets
|
||||
|
||||
## Description
|
||||
|
||||
`SecretStrSubstNo` is the secret-preserving counterpart of `StrSubstNo`. It accepts a format string and arguments (any of which may be `SecretText`) and returns a `SecretText` — the substitution happens without ever materializing the result as plain `Text`. It is the right tool whenever a secret needs to be embedded in a larger string: an `Authorization: Bearer <token>` header value, a URI that includes an API key as a query parameter, or any other interpolation that combines a `SecretText` with surrounding context.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Compose every secret-bearing string through `SecretStrSubstNo` and keep the result as `SecretText` end-to-end. Pass the result to the `SecretText` overload of the consumer — `HttpClient.SetSecretRequestUri`, `HttpHeaders.Add`, or `HttpContent.WriteFrom`. See sample: `secretstrsubstno-for-composing-secrets.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling `StrSubstNo('Bearer %1', Token.Unwrap())` to build the header value, or concatenating `'Bearer ' + Token.Unwrap()`. Both produce a plain `Text` containing the secret, which is then visible in the debugger and in any subsequent log or trace. Reviewers should flag any `Unwrap()` whose result is fed into `StrSubstNo` or used in `+` concatenation — `SecretStrSubstNo` removes the need for either. See sample: `secretstrsubstno-for-composing-secrets.bad.al`.
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
codeunit 50208 "Sec Sample SecretText Bad"
|
||||
{
|
||||
procedure CallExternalApi()
|
||||
var
|
||||
ApiKey: Text;
|
||||
BearerToken: Text;
|
||||
HttpClient: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
Headers: HttpHeaders;
|
||||
begin
|
||||
ApiKey := GetApiKey();
|
||||
BearerToken := GetAccessToken();
|
||||
Headers := HttpClient.DefaultRequestHeaders();
|
||||
Headers.Add('Authorization', 'Bearer ' + BearerToken);
|
||||
Headers.Add('X-Api-Key', ApiKey);
|
||||
HttpClient.Get('https://api.example.com/data', Response);
|
||||
end;
|
||||
|
||||
local procedure GetApiKey(): Text begin end;
|
||||
|
||||
local procedure GetAccessToken(): Text begin end;
|
||||
}
|
||||
|
|
@ -0,0 +1,16 @@
|
|||
codeunit 50207 "Sec Sample SecretText Good"
|
||||
{
|
||||
procedure CallExternalApi()
|
||||
var
|
||||
ApiKey: SecretText;
|
||||
HttpClient: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
Headers: HttpHeaders;
|
||||
begin
|
||||
if IsolatedStorage.Contains('ApiKey', DataScope::Module) then
|
||||
IsolatedStorage.Get('ApiKey', DataScope::Module, ApiKey);
|
||||
Headers := HttpClient.DefaultRequestHeaders();
|
||||
Headers.Add('X-Api-Key', ApiKey);
|
||||
HttpClient.Get('https://api.example.com/data', Response);
|
||||
end;
|
||||
}
|
||||
22
microsoft/knowledge/security/secrettext-for-credentials.md
Normal file
22
microsoft/knowledge/security/secrettext-for-credentials.md
Normal file
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [secrettext, credentials, api-key, token, debugger, unwrap]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use SecretText for credentials, API keys, and tokens
|
||||
|
||||
## Description
|
||||
|
||||
`SecretText` is the AL data type for values that should never appear in a debugger session, in a log, or in a variable watch. The compiler enforces two guarantees: a string literal cannot be assigned directly to a `SecretText` variable, and a `SecretText` cannot be assigned back to a `Text` or `Code` without an explicit `Unwrap` call. Together these prevent the two common accidents — embedding a secret in source code, and quietly converting a secret to plain text where the debugger can read it. Use `SecretText` for parameters, return values, and local variables that carry API keys, tokens, passwords, connection strings, or any other value an attacker with debugger access should not see.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Declare credential-carrying parameters and variables as `SecretText` from the call site that retrieves the secret all the way to the call site that consumes it (typically an `HttpClient` header or URI). Never round-trip through `Text` — every conversion is a potential exposure point. Retrieve secrets from `IsolatedStorage` with the `SecretText` overload of `Get` rather than the `Text` overload. See sample: `secrettext-for-credentials.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Holding a credential in a `Text` variable (`BearerToken: Text`), concatenating it into a header, then passing it to `HttpClient`. The token is visible in the debugger and in any error that prints the variable, and the compiler offers no help because the type was wrong from the start. Reviewers should flag any local or parameter named like a secret (`ApiKey`, `Token`, `Password`, `ClientSecret`) whose type is `Text` or `Code`. See sample: `secrettext-for-credentials.bad.al`.
|
||||
|
|
@ -0,0 +1,23 @@
|
|||
codeunit 50210 "Sec Sample SecretHttp Bad"
|
||||
{
|
||||
procedure CallApiWithSecretInUri(ApiKey: SecretText)
|
||||
var
|
||||
HttpClient: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
RequestUri: Text;
|
||||
begin
|
||||
RequestUri := 'https://api.example.com/data?key=' + ApiKey.Unwrap();
|
||||
HttpClient.Get(RequestUri, Response);
|
||||
end;
|
||||
|
||||
procedure CallApiWithBearer(BearerToken: SecretText)
|
||||
var
|
||||
HttpClient: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
Headers: HttpHeaders;
|
||||
begin
|
||||
Headers := HttpClient.DefaultRequestHeaders();
|
||||
Headers.Add('Authorization', 'Bearer ' + BearerToken.Unwrap());
|
||||
HttpClient.Get('https://api.example.com/data', Response);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
codeunit 50209 "Sec Sample SecretHttp Good"
|
||||
{
|
||||
procedure CallApiWithSecretUri(ApiKey: SecretText)
|
||||
var
|
||||
HttpClient: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
SecretUri: SecretText;
|
||||
begin
|
||||
SecretUri := SecretStrSubstNo('https://api.example.com/data?key=%1', ApiKey);
|
||||
HttpClient.SetSecretRequestUri(SecretUri);
|
||||
HttpClient.Get('', Response);
|
||||
end;
|
||||
|
||||
procedure CallApiWithBearer(BearerToken: SecretText)
|
||||
var
|
||||
HttpClient: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
Headers: HttpHeaders;
|
||||
AuthHeader: SecretText;
|
||||
begin
|
||||
AuthHeader := SecretStrSubstNo('Bearer %1', BearerToken);
|
||||
Headers := HttpClient.DefaultRequestHeaders();
|
||||
Headers.Add('Authorization', AuthHeader);
|
||||
if not Headers.ContainsSecret('Authorization') then
|
||||
Error('Authorization header missing');
|
||||
HttpClient.Get('https://api.example.com/data', Response);
|
||||
end;
|
||||
}
|
||||
22
microsoft/knowledge/security/secrettext-with-httpclient.md
Normal file
22
microsoft/knowledge/security/secrettext-with-httpclient.md
Normal file
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [secrettext, httpclient, setsecretrequesturi, containssecret, headers, http]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use the SecretText-aware HttpClient surface for secrets in requests
|
||||
|
||||
## Description
|
||||
|
||||
`HttpClient` and its companion types expose a parallel surface that accepts `SecretText` instead of `Text`, so that secret URIs, secret headers, and secret request bodies never round-trip through plain text. The key entry points are: `HttpClient.SetSecretRequestUri()` for URIs that contain secrets (the subsequent `Get`/`Post` is then called with an empty string); `HttpHeaders.Add()` overload that accepts a `SecretText` value for authorization headers; `HttpHeaders.ContainsSecret()` to test whether a secret header is present (the plain `Contains()` returns false for secret headers); `HttpContent.WriteFrom()` and `HttpContent.ReadAs()` overloads that accept and produce `SecretText` for request and response bodies that carry credentials.
|
||||
|
||||
## Best Practice
|
||||
|
||||
When the URI contains a secret query parameter, compose it as `SecretText` (see `secretstrsubstno-for-composing-secrets.md`), pass it to `SetSecretRequestUri`, and call `Get('', Response)` with an empty string as the URI argument. When the credential is an authorization header, build the header value as `SecretText` and pass it to `Headers.Add`. Use `ContainsSecret` rather than `Contains` to check for the presence of a secret header. See sample: `secrettext-with-httpclient.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling `ApiKey.Unwrap()` to build a URI or header string and passing the resulting `Text` to `HttpClient.Get` or `Headers.Add`. The unwrapped secret is now visible in the debugger, in any HTTP trace that captures the request URI, and in any error that includes the URI. Reviewers should flag any `Unwrap()` call whose result flows into an `HttpClient` argument; the `SecretText` overload exists precisely so the unwrap is not needed. See sample: `secrettext-with-httpclient.bad.al`.
|
||||
|
|
@ -1,7 +0,0 @@
|
|||
permissionset 50203 "Sec Sample Direct Write"
|
||||
{
|
||||
Assignable = true;
|
||||
Caption = 'Direct write granted to every caller (sample anti-pattern)';
|
||||
Permissions =
|
||||
tabledata "Sales Header" = RM;
|
||||
}
|
||||
|
|
@ -1,34 +0,0 @@
|
|||
permissionset 50202 "Sec Sample Elevated Write"
|
||||
{
|
||||
Assignable = false;
|
||||
Caption = 'Elevated write via helper (sample)';
|
||||
// Callers hold R directly; the helper codeunit assumes this set and
|
||||
// performs the Modify via indirect permission.
|
||||
Permissions =
|
||||
tabledata "Sales Header" = Rmi;
|
||||
}
|
||||
|
||||
codeunit 50231 "Sec Sample Elevated Helper"
|
||||
{
|
||||
Access = Public;
|
||||
Permissions = tabledata "Sales Header" = Rmi;
|
||||
|
||||
procedure SetExternalDocumentNo(SalesDocType: Enum "Sales Document Type"; SalesDocNo: Code[20]; NewExternalDocNo: Code[35])
|
||||
var
|
||||
SalesHeader: Record "Sales Header";
|
||||
begin
|
||||
ValidateCaller();
|
||||
if NewExternalDocNo = '' then
|
||||
Error('External document number must be provided.');
|
||||
if not SalesHeader.Get(SalesDocType, SalesDocNo) then
|
||||
Error('Sales document not found.');
|
||||
SalesHeader."External Document No." := NewExternalDocNo;
|
||||
SalesHeader.Modify(true);
|
||||
end;
|
||||
|
||||
local procedure ValidateCaller()
|
||||
begin
|
||||
// Verify the caller is permitted to perform this elevated write
|
||||
// (role check, setup flag, approvals, etc.).
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,29 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [indirect-permission, elevation, permissionset]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use indirect permissions for elevated access
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
Indirect permissions (ri, ii, mi, di) let a procedure perform an operation against tabledata the caller does not have direct rights to, provided the caller is authorized to invoke the procedure. They are the supported mechanism for elevation: instead of widening every caller's direct rights to M or D, the sensitive operation lives in a codeunit that holds the indirect right and validates its callers.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Where a module exposes a controlled write or delete against a sensitive table, grant the codeunit (or the helper permission set it assumes) the indirect permission (mi, di) it requires, keep direct permissions minimal, and document why the elevation is justified. The helper MUST validate its inputs and the caller's identity before performing the elevated work.
|
||||
|
||||
See sample: `use-indirect-permissions-for-elevated-access.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Granting direct M or D on a sensitive tabledata to every role that might invoke a helper, because authoring an indirect-permission codeunit was inconvenient. Every caller now has the elevated right for every code path, not just the one the helper implements.
|
||||
|
||||
See sample: `use-indirect-permissions-for-elevated-access.bad.al`.
|
||||
|
||||
|
|
@ -1,13 +0,0 @@
|
|||
codeunit 50205 "Sec Sample Inherent Bad"
|
||||
{
|
||||
// No InherentPermissions attribute: every caller must hold
|
||||
// tabledata "Sec Sample Lookup" = R just to look up a name.
|
||||
procedure GetLookupName(LookupCode: Code[20]): Text[100]
|
||||
var
|
||||
Lookup: Record "Sec Sample Lookup";
|
||||
begin
|
||||
if Lookup.Get(LookupCode) then
|
||||
exit(Lookup.Name);
|
||||
exit('');
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,28 +0,0 @@
|
|||
table 50230 "Sec Sample Lookup"
|
||||
{
|
||||
DataClassification = SystemMetadata;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Code"; Code[20]) { }
|
||||
field(2; "Name"; Text[100]) { }
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Code") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50204 "Sec Sample Inherent Good"
|
||||
{
|
||||
[InherentPermissions(PermissionObjectType::TableData, Database::"Sec Sample Lookup", 'r')]
|
||||
procedure GetLookupName(LookupCode: Code[20]): Text[100]
|
||||
var
|
||||
Lookup: Record "Sec Sample Lookup";
|
||||
begin
|
||||
if Lookup.Get(LookupCode) then
|
||||
exit(Lookup.Name);
|
||||
exit('');
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,29 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [inherentpermissions, attribute, least-privilege]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use InherentPermissions to grant minimal access
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
The InherentPermissions attribute attaches a minimum access grant to a procedure. Callers can invoke the procedure without holding the underlying tabledata right, because the attribute supplies exactly the right required by the procedure body and nothing more. InherentPermissions currently targets only objects owned by the same extension as the annotated procedure; it cannot be used to grant access to tables in other extensions or in the base application.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Annotate read-only helper procedures with InherentPermissions specifying only the tables and access letters the body uses (typically 'r'). Callers do not need direct read rights on the underlying extension-owned table, so the calling role can be narrower. This is the narrowest of the elevation options and is appropriate for read-only lookup helpers.
|
||||
|
||||
See sample: `use-inherent-permissions-to-grant-minimal-access.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A helper that reads a single lookup value but forces every calling role to hold tabledata read rights, because the helper does not declare its own inherent permissions. The broad read right then applies to every other code path that role can reach, not just the helper.
|
||||
|
||||
See sample: `use-inherent-permissions-to-grant-minimal-access.bad.al`.
|
||||
|
||||
|
|
@ -1,18 +0,0 @@
|
|||
codeunit 50209 "Sec Sample IsolatedStorage Bad"
|
||||
{
|
||||
procedure StoreApiKey(NewKey: Text)
|
||||
begin
|
||||
// Plaintext write to IsolatedStorage is not encrypted at rest.
|
||||
IsolatedStorage.Set('ApiKey', NewKey, DataScope::Module);
|
||||
end;
|
||||
|
||||
procedure GetApiKey(): Text
|
||||
var
|
||||
ApiKey: Text;
|
||||
begin
|
||||
// Public wrapper: another extension can call this to read the secret.
|
||||
if IsolatedStorage.Get('ApiKey', DataScope::Module, ApiKey) then
|
||||
exit(ApiKey);
|
||||
exit('');
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,14 +0,0 @@
|
|||
codeunit 50208 "Sec Sample IsolatedStorage Good"
|
||||
{
|
||||
internal procedure StoreApiKey(NewKey: SecretText)
|
||||
begin
|
||||
IsolatedStorage.SetEncrypted('ApiKey', NewKey, DataScope::Module);
|
||||
end;
|
||||
|
||||
local procedure TryGetApiKey(var ApiKey: SecretText): Boolean
|
||||
begin
|
||||
if IsolatedStorage.Contains('ApiKey', DataScope::Module) then
|
||||
exit(IsolatedStorage.Get('ApiKey', DataScope::Module, ApiKey));
|
||||
exit(false);
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,29 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [isolatedstorage, encryption, datascope, secrets]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use IsolatedStorage for module and company secrets
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
IsolatedStorage is a per-extension, per-tenant key-value store. DataScope::Module isolates values to the extension across the tenant; DataScope::Company scopes them to a single company within the tenant. The SetEncrypted method stores the value encrypted at rest; Set stores it in plaintext. SetEncrypted accepts inputs up to 215 characters (special characters may consume more space).
|
||||
|
||||
## Best Practice
|
||||
|
||||
Use IsolatedStorage.SetEncrypted to write secrets, IsolatedStorage.Contains to probe, and IsolatedStorage.Get into a SecretText destination to read. Choose DataScope::Company for per-company credentials (for example, a tenant-per-company service account) and DataScope::Module for extension-wide configuration. Procedures that call IsolatedStorage.Get, Set, SetEncrypted, Contains, or Delete must be `local` or `internal`; a public wrapper lets other extensions call into your storage boundary.
|
||||
|
||||
See sample: `use-isolated-storage-for-module-and-company-secrets.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Storing secrets in a Setup table column as plain Text, using IsolatedStorage.Set (unencrypted) for values that authenticate the extension to an external service, or exposing a public Get/Set procedure around IsolatedStorage. The first two leave secrets readable; the public wrapper lets another extension exfiltrate or overwrite values through your codeunit.
|
||||
|
||||
See sample: `use-isolated-storage-for-module-and-company-secrets.bad.al`.
|
||||
|
||||
|
|
@ -1,21 +0,0 @@
|
|||
codeunit 50217 "Sec Sample NonDebuggable Bad"
|
||||
{
|
||||
// Missing [NonDebuggable]: ResponseText and the extracted token are
|
||||
// inspectable in the debugger and in snapshot debug sessions.
|
||||
procedure ParseSessionToken(Response: HttpResponseMessage; var SessionToken: SecretText)
|
||||
var
|
||||
ResponseText: Text;
|
||||
JObject: JsonObject;
|
||||
JToken: JsonToken;
|
||||
begin
|
||||
Response.Content.ReadAs(ResponseText);
|
||||
JObject.ReadFrom(ResponseText);
|
||||
JObject.Get('access_token', JToken);
|
||||
SessionToken := JToken.AsValue().AsText();
|
||||
end;
|
||||
|
||||
procedure BuildAuthorizationHeader(ApiKey: SecretText): Text
|
||||
begin
|
||||
exit('Bearer ' + ApiKey.Unwrap());
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,21 +0,0 @@
|
|||
codeunit 50216 "Sec Sample NonDebuggable Good"
|
||||
{
|
||||
[NonDebuggable]
|
||||
procedure ParseSessionToken(Response: HttpResponseMessage; var SessionToken: SecretText)
|
||||
var
|
||||
ResponseText: Text;
|
||||
JObject: JsonObject;
|
||||
JToken: JsonToken;
|
||||
begin
|
||||
Response.Content.ReadAs(ResponseText);
|
||||
JObject.ReadFrom(ResponseText);
|
||||
JObject.Get('access_token', JToken);
|
||||
SessionToken := JToken.AsValue().AsText();
|
||||
end;
|
||||
|
||||
[NonDebuggable]
|
||||
procedure BuildAuthorizationHeader(ApiKey: SecretText): Text
|
||||
begin
|
||||
exit('Bearer ' + ApiKey.Unwrap());
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,29 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [nondebuggable, secrettext, attribute, parse]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use NonDebuggable when parsing secrets
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
SecretText transit (assignment between SecretText variables, parameters, and return values) is protected automatically. Extracting a secret from a Text source — for example, reading an access token out of a parsed JSON response — is a legitimate Text-to-SecretText conversion during which the plaintext exists. Calling `SecretText.Unwrap()` has the same exposure in the opposite direction: it materializes the secret as plain Text. The [NonDebuggable] attribute prevents debuggers (regular and snapshot) from inspecting the procedure's locals, parameters, and return at that moment.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Apply [NonDebuggable] to any procedure that reads a response body, parses it, and assigns the extracted secret to a SecretText out-parameter or return. Also apply it to every procedure that calls `Unwrap()` because the secret becomes plain Text inside that procedure. Keep the procedure narrow: it SHOULD do the minimum work required to obtain or unwrap the secret, and nothing else.
|
||||
|
||||
See sample: `use-nondebuggable-when-parsing-secrets.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Parsing a token response in a normal (debuggable) procedure, or calling `ApiKey.Unwrap()` there to build a legacy Text value. The plaintext token is visible in debug sessions and snapshots taken during the parse or unwrap.
|
||||
|
||||
See sample: `use-nondebuggable-when-parsing-secrets.bad.al`.
|
||||
|
||||
|
|
@ -1,13 +0,0 @@
|
|||
codeunit 50211 "Sec Sample SecretText Bad"
|
||||
{
|
||||
procedure SendAuthenticatedRequest(BearerToken: Text)
|
||||
var
|
||||
Client: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
AuthValue: Text;
|
||||
begin
|
||||
AuthValue := 'Bearer ' + BearerToken;
|
||||
Client.DefaultRequestHeaders.Add('Authorization', AuthValue);
|
||||
Client.Get('https://api.example.com/data', Response);
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,14 +0,0 @@
|
|||
codeunit 50210 "Sec Sample SecretText Good"
|
||||
{
|
||||
procedure SendAuthenticatedRequest(BearerToken: SecretText)
|
||||
var
|
||||
Client: HttpClient;
|
||||
Headers: HttpHeaders;
|
||||
Response: HttpResponseMessage;
|
||||
AuthValue: SecretText;
|
||||
begin
|
||||
AuthValue := SecretStrSubstNo('Bearer %1', BearerToken);
|
||||
Client.DefaultRequestHeaders.Add('Authorization', AuthValue);
|
||||
Client.Get('https://api.example.com/data', Response);
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,27 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [secrettext, credentials, debugger, type]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use SecretText for credentials
|
||||
|
||||
## Description
|
||||
|
||||
SecretText is a compile-time-checked AL type for credentials, API keys, tokens, and similar sensitive values. The compiler rejects literal assignments to SecretText and blocks implicit conversion back to Text or Code, which prevents many accidental disclosures via logs, errors, and the debugger (regular and snapshot). A SecretText value remains opaque throughout its lifetime.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Type every credential-carrying variable, procedure parameter, and return as SecretText. Compose values with SecretStrSubstNo (see compose-secrets-with-secretstrsubstno). For HttpClient integration, see use-secrettext-with-httpclient. When a secret must be extracted from a Text source, contain that conversion in a NonDebuggable procedure (see use-nondebuggable-when-parsing-secrets).
|
||||
|
||||
See sample: `use-secrettext-for-credentials.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Passing credentials around as Text or Code parameters. Every such variable is visible in the debugger and may be captured by error handlers, logs, and telemetry that treat Text as non-sensitive.
|
||||
|
||||
See sample: `use-secrettext-for-credentials.bad.al`.
|
||||
|
||||
|
|
@ -1,12 +0,0 @@
|
|||
codeunit 50213 "Sec Sample SecretHttpClient Bad"
|
||||
{
|
||||
procedure Call(ApiKey: Text)
|
||||
var
|
||||
Client: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
FullUrl: Text;
|
||||
begin
|
||||
FullUrl := 'https://api.example.com/v1?key=' + ApiKey;
|
||||
Client.Get(FullUrl, Response);
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,15 +0,0 @@
|
|||
codeunit 50212 "Sec Sample SecretHttpClient Good"
|
||||
{
|
||||
procedure Call(ApiKey: SecretText)
|
||||
var
|
||||
Client: HttpClient;
|
||||
Request: HttpRequestMessage;
|
||||
Response: HttpResponseMessage;
|
||||
SecretUri: SecretText;
|
||||
begin
|
||||
SecretUri := SecretStrSubstNo('https://api.example.com/v1?key=%1', ApiKey);
|
||||
Request.SetSecretRequestUri(SecretUri);
|
||||
Request.Method('GET');
|
||||
Client.Send(Request, Response);
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,27 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [httpclient, secrettext, headers, uri]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use SecretText with HttpClient
|
||||
|
||||
## Description
|
||||
|
||||
HttpRequestMessage, HttpHeaders, and HttpContent expose SecretText overloads so credentials never have to be converted back to Text to be sent. Key APIs: HttpRequestMessage.SetSecretRequestUri (for URIs containing secrets), HttpHeaders.Add(name, SecretText) for authorization headers, HttpHeaders.ContainsSecret to probe secret-valued headers, HttpContent.WriteFrom(SecretText) for request bodies, and HttpContent.ReadAs(SecretText) to pull response bodies into a secret destination.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Use HttpRequestMessage.SetSecretRequestUri when any URI component is sensitive (for example, a per-call API key in the path or query), and send the request with HttpClient.Send. Add Authorization headers as SecretText. Check for the presence of a secret header with ContainsSecret, not Contains.
|
||||
|
||||
See sample: `use-secrettext-with-httpclient.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Materializing a URI or header value as Text to 'just get it to compile' — for example, StrSubstNo into a Text and then HttpClient.Get(FullUrl, Response). The resulting Text is visible in debuggers, and the URL is typically captured by platform-level logging the extension does not control.
|
||||
|
||||
See sample: `use-secrettext-with-httpclient.bad.al`.
|
||||
|
||||
|
|
@ -1,10 +0,0 @@
|
|||
codeunit 50241 "Sec Sample Url Bad"
|
||||
{
|
||||
procedure Sync(ServiceUrl: Text)
|
||||
var
|
||||
Client: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
begin
|
||||
Client.Get(ServiceUrl, Response);
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,17 +0,0 @@
|
|||
codeunit 50240 "Sec Sample Url Good"
|
||||
{
|
||||
procedure Sync(ServiceUrl: Text)
|
||||
var
|
||||
Client: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
Uri: Codeunit Uri;
|
||||
ExpectedBaseUrl: Text;
|
||||
begin
|
||||
ExpectedBaseUrl := 'https://api.contoso.com';
|
||||
|
||||
if not Uri.AreURIsHaveSameHost(ServiceUrl, ExpectedBaseUrl) then
|
||||
Error('Service URL must point to api.contoso.com.');
|
||||
|
||||
Client.Get(ServiceUrl, Response);
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,26 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [url, uri, httpclient, ssrf, validation, endpoint]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Validate user-configurable URLs before HTTP calls
|
||||
|
||||
## Description
|
||||
|
||||
URLs stored in setup tables or accepted from user input are user-configurable endpoints. Passing them directly to `HttpClient` lets a malicious or compromised setup value redirect the extension to internal services, metadata endpoints, or attacker-controlled hosts. Business Central's System Application `Uri` codeunit provides host and pattern validation helpers for this exact boundary.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Before `HttpClient.Get`, `Post`, `Put`, or similar calls use a URL from a table field, validate it with `Uri.AreURIsHaveSameHost()` when the host must be fixed, or `Uri.IsValidURIPattern()` when a known URL pattern is allowed. Validate before writing the request body so sensitive payloads are never sent to an unexpected host.
|
||||
|
||||
See sample: `validate-user-configurable-urls-before-http-calls.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Reading `Setup."Service URL"` or `WebhookSetup."Callback URL"` and passing it directly to HttpClient. The code looks configurable, but it creates an SSRF path and can exfiltrate data to whichever host the setup row names.
|
||||
|
||||
See sample: `validate-user-configurable-urls-before-http-calls.bad.al`.
|
||||
|
|
@ -0,0 +1,20 @@
|
|||
codeunit 50222 "Sec Sample UrlValidation Bad"
|
||||
{
|
||||
procedure SyncWithExternalService(ServiceUrl: Text)
|
||||
var
|
||||
HttpClient: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
begin
|
||||
HttpClient.Get(ServiceUrl, Response);
|
||||
end;
|
||||
|
||||
procedure SendWebhookNotification(CallbackUrl: Text; Payload: Text)
|
||||
var
|
||||
HttpClient: HttpClient;
|
||||
Content: HttpContent;
|
||||
Response: HttpResponseMessage;
|
||||
begin
|
||||
Content.WriteFrom(Payload);
|
||||
HttpClient.Post(CallbackUrl, Content, Response);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,24 @@
|
|||
codeunit 50221 "Sec Sample UrlValidation Good"
|
||||
{
|
||||
procedure SyncWithExternalService(ServiceUrl: Text)
|
||||
var
|
||||
HttpClient: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
Uri: Codeunit Uri;
|
||||
begin
|
||||
if not Uri.AreURIsHaveSameHost(ServiceUrl, 'https://api.contoso.com') then
|
||||
Error('Service URL must point to api.contoso.com');
|
||||
HttpClient.Get(ServiceUrl, Response);
|
||||
end;
|
||||
|
||||
procedure SyncWithShopify(ShopUrl: Text)
|
||||
var
|
||||
HttpClient: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
Uri: Codeunit Uri;
|
||||
begin
|
||||
if not Uri.IsValidURIPattern(ShopUrl, 'https://*.myshopify.com/*') then
|
||||
Error('Shop URL must match the Shopify pattern');
|
||||
HttpClient.Get(ShopUrl + '/admin/api/2024-01/orders.json', Response);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [ssrf, uri, url-validation, areurishavesamehost, isvaliduripattern, httpclient]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Validate URLs that come from table fields before calling them
|
||||
|
||||
## Description
|
||||
|
||||
A URL stored in a table field is user-configurable: anyone with write access to the row can change it. If that URL is then used as the target of an `HttpClient.Get`/`Post`, the extension becomes a server-side request forgery (SSRF) primitive — an attacker can redirect the call to an internal endpoint, to a metadata service, or to a malicious host that mirrors the legitimate API. The `Uri` codeunit from System Modules provides two validators built for this situation: `AreURIsHaveSameHost()` checks that two URLs share the same host (use when the hostname should not change — for example, the extension always talks to `api.contoso.com`). `IsValidURIPattern()` checks that a URL matches a wildcard pattern (use when the host varies but follows a predictable shape — for example `https://{store}.myshopify.com/...`).
|
||||
|
||||
## Best Practice
|
||||
|
||||
Before any `HttpClient` call whose URL came from a table field, call `Uri.AreURIsHaveSameHost(StoredUrl, ExpectedBaseUrl)` against a hard-coded expected base, or `Uri.IsValidURIPattern(StoredUrl, 'https://*.myshopify.com/*')` against a fixed pattern. Fail the call with an `Error` when the validator returns false. For webhook scenarios where the host is registered out-of-band, compare against the registered host stored alongside the URL. See sample: `validate-user-configurable-urls.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`HttpClient.Get(Setup."Service URL", Response)` or `HttpClient.Post(WebhookSetup."Callback URL", Content, Response)` with no validation step in between. The extension will dutifully send the request — and any sensitive payload — to whatever host the attacker put in the field. Reviewers should flag any `HttpClient` call whose first argument is a record field, an `OnValidate`-mutable field, or a value sourced from a table read, unless a `Uri.AreURIsHaveSameHost` or `Uri.IsValidURIPattern` check precedes it. See sample: `validate-user-configurable-urls.bad.al`.
|
||||
|
|
@ -0,0 +1,11 @@
|
|||
tableextension 50225 "Sec Sample VTR Bad" extends Customer
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(50225; "Linked Customer No."; Code[20])
|
||||
{
|
||||
TableRelation = Customer."No.";
|
||||
ValidateTableRelation = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
tableextension 50223 "Sec Sample VTR Good" extends Customer
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(50223; "System Batch ID"; Code[20])
|
||||
{
|
||||
TableRelation = "Sales Header"."No.";
|
||||
ValidateTableRelation = false;
|
||||
Editable = false;
|
||||
}
|
||||
field(50224; "External Customer Ref"; Code[50])
|
||||
{
|
||||
TableRelation = Customer."No.";
|
||||
ValidateTableRelation = false;
|
||||
trigger OnValidate()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
if "External Customer Ref" = '' then
|
||||
exit;
|
||||
if not Customer.Get("External Customer Ref") then
|
||||
Error('External customer reference %1 does not exist.', "External Customer Ref");
|
||||
end;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [validatetablerelation, tablerelation, field, validation, input]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not set ValidateTableRelation = false on user-editable fields
|
||||
|
||||
## Description
|
||||
|
||||
`TableRelation` on a field declares that the field's value must exist in another table; the platform validates the value on entry and on `Validate`. Setting `ValidateTableRelation = false` keeps the relation as metadata (used by lookups, by Edit-in-Excel, by APIs) but turns off the runtime check. On a system-controlled, non-editable field that is populated only by the platform or by a posting routine, that is acceptable. On a user-editable field, it is dangerous: users can type any value, and downstream code that assumes the relation holds will read a `Customer` record that does not exist, post to an account that was deleted, or join against missing rows.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Leave `ValidateTableRelation` at its default (true) on any field a user can edit. If there is a legitimate reason to turn it off — typically because the relation is not on the primary key, or because the relation is computed — replace it with an `OnValidate` trigger that performs the equivalent check (`if FieldValue <> '' then VerifyExternalReferenceExists(FieldValue)`). Combine `ValidateTableRelation = false` with `Editable = false` for system-controlled fields, so the metadata is correct and the field is unreachable from the UI. See sample: `validatetablerelation-false-on-user-input.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`ValidateTableRelation = false` on a user-facing input field (a `Customer No.` typed by a sales user) with no alternative validation. Reviewers should flag the combination of `ValidateTableRelation = false` and any of: `Editable = true` (the default), an `OnValidate` trigger that does not perform the relation check, or a page that surfaces the field as input. See sample: `validatetablerelation-false-on-user-input.bad.al`.
|
||||
Loading…
Add table
Add a link
Reference in a new issue