mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 14:46:55 +01:00
Improve partner onboarding and documentation navigation
Lead with a complete plugin quick start and add task-oriented usage, troubleshooting, customization, and contribution guides. Preserve the broader plugin framing, correct conflicting contract guidance, support Agents folder reviews, and align repository validation. Convert existing sample references to clickable links without changing knowledge rules. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Jesper Schulz-Wedde
parent
a21edfec46
commit
b6da405376
276 changed files with 1287 additions and 756 deletions
|
|
@ -15,8 +15,8 @@ AL does not ship a built-in `HtmlEncode` (or equivalent) function. Code that bui
|
|||
|
||||
## 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`.
|
||||
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`](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`.
|
||||
`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`](al-has-no-built-in-htmlencode.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,10 +17,10 @@ application-area: [all]
|
|||
|
||||
Annotate publisher methods whose transactional guarantees must survive extension code. The attribute is a selective guard, not a convention: most `IntegrationEvent` publishers do not need it. Events that fire from a standalone query, events fired after the publisher has already committed, informational hooks, and notification-style events are unaffected by subscriber commits. Reach for the attribute only when the publisher has uncommitted writes at the moment of firing and a premature inner commit would persist inconsistent state. Prefer `Ignore` over `Error` when the intent is "silently nullify" — an `Error` from an extension's commit would surface as a subscriber-authored dialog rather than a publisher-defined failure mode. Pair the attribute with the actual atomic-boundary logic in the publisher (validate, then `Commit` on success); a subscriber's suppressed commit remains a no-op regardless of how the publisher completes.
|
||||
|
||||
See sample: `commitbehavior-attribute-scopes-explicit-commits.good.al`.
|
||||
See sample: [`commitbehavior-attribute-scopes-explicit-commits.good.al`](commitbehavior-attribute-scopes-explicit-commits.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Publishing an `IntegrationEvent` from inside an atomic operation without `[CommitBehavior(CommitBehavior::Ignore)]`. A third-party subscriber that calls `Commit()` — intentionally or by accident — persists the publisher's partial state, defeating any rollback the publisher would have performed on a later validation failure. Another anti-pattern is placing the attribute on a wrapper method and calling a nested `Codeunit.Run` that writes, expecting the attribute to suppress the implicit commit: it does not. The mirror-image anti-pattern is applying the attribute reflexively to every `IntegrationEvent` regardless of context — events that fire outside an atomic sequence gain nothing from the protection, and adding it everywhere clutters the review surface and masks the publishers that genuinely need it.
|
||||
|
||||
See sample: `commitbehavior-attribute-scopes-explicit-commits.bad.al`.
|
||||
See sample: [`commitbehavior-attribute-scopes-explicit-commits.bad.al`](commitbehavior-attribute-scopes-explicit-commits.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,10 +17,10 @@ The `IncludedPermissionSets` property lets one AL permission set reference anoth
|
|||
|
||||
Break permission grants into small, focused building blocks, one per cohesive concern. Mark the building blocks `Assignable = false` so administrators do not accidentally assign a fragment. Build role-shaped, `Assignable = true` sets that reference the relevant building blocks through `IncludedPermissionSets`. When the extension grows, the structure absorbs the growth without duplicated edits.
|
||||
|
||||
See sample: `compose-permission-sets-with-included-sets.good.al`.
|
||||
See sample: [`compose-permission-sets-with-included-sets.good.al`](compose-permission-sets-with-included-sets.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Declaring several role-shaped permission sets that each re-enumerate the same object lists. Adding a new table means touching every set by hand; the sets drift apart over time, and subtle authorization bugs appear where one role was updated and a sibling role was not.
|
||||
|
||||
See sample: `compose-permission-sets-with-included-sets.bad.al`.
|
||||
See sample: [`compose-permission-sets-with-included-sets.bad.al`](compose-permission-sets-with-included-sets.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ It is tempting to flag any code that calls `GetLastErrorText()` and writes the r
|
|||
|
||||
## 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.
|
||||
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`](getlasterrortext-storage-is-privacy-not-security.bad.al) for the pattern that is *not* a security finding.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
|
|
|
|||
|
|
@ -19,10 +19,10 @@ An AL helper that accepts a `var Rec: Record X` parameter and performs a bulk op
|
|||
|
||||
Any helper designed to operate on a temporary record, and that performs `DeleteAll`, `ModifyAll`, or similar bulk writes on its parameter, should call `Rec.IsTemporary()` at the top and raise a descriptive error when the assumption is violated. The error message should name the parameter so the misuse is easy to locate.
|
||||
|
||||
See sample: `guard-bulk-operations-with-istemporary.good.al`.
|
||||
See sample: [`guard-bulk-operations-with-istemporary.good.al`](guard-bulk-operations-with-istemporary.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Trusting documentation or naming conventions alone to signal that a `var Rec` parameter is expected to be temporary. A future refactor or a copy-paste caller can pass the real table; the bulk operation then executes against production rows silently.
|
||||
|
||||
See sample: `guard-bulk-operations-with-istemporary.bad.al`.
|
||||
See sample: [`guard-bulk-operations-with-istemporary.bad.al`](guard-bulk-operations-with-istemporary.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ In a `permissionset`, uppercase letters (`R`, `I`, `M`, `D`) grant **direct** pe
|
|||
|
||||
## 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`.
|
||||
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`](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`.
|
||||
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`](indirect-permissions-for-elevated-access.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ application-area: [all]
|
|||
|
||||
## 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`.
|
||||
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`](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`.
|
||||
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`](inherent-permissions-minimal-grant.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ application-area: [all]
|
|||
|
||||
## 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`.
|
||||
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`](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`.
|
||||
`[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`](integrationevent-must-not-expose-secrets.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ A `var` parameter on an `[IntegrationEvent]` is a mutable hook: any subscriber c
|
|||
|
||||
## 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`.
|
||||
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`](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`.
|
||||
`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`](integrationevent-var-parameter-bypasses-security-guards.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,10 +17,10 @@ application-area: [all]
|
|||
|
||||
Use `internal` to keep implementation details out of the supported API, but enforce sensitive operations with permissions, entitlements, and explicit authorization checks appropriate to the operation. Treat `internalsVisibleTo` as a same-publisher development/testability relationship, not as a trust grant for secrets or elevated data access.
|
||||
|
||||
See sample: `internal-access-is-not-a-security-boundary.good.al`.
|
||||
See sample: [`internal-access-is-not-a-security-boundary.good.al`](internal-access-is-not-a-security-boundary.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Placing privileged work in an internal codeunit and claiming that other extensions cannot invoke it, or exposing an app to a different publisher through `internalsVisibleTo` because `internal` is assumed to protect the underlying operation. The access modifier narrows supported callers; it does not authenticate runtime callers.
|
||||
|
||||
See sample: `internal-access-is-not-a-security-boundary.bad.al`.
|
||||
See sample: [`internal-access-is-not-a-security-boundary.bad.al`](internal-access-is-not-a-security-boundary.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ application-area: [all]
|
|||
|
||||
## 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`.
|
||||
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`](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`.
|
||||
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`](isolatedstorage-access-must-be-local-or-internal.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ application-area: [all]
|
|||
|
||||
## 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`.
|
||||
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`](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`.
|
||||
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`](isolatedstorage-datascope-module-vs-company.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ application-area: [all]
|
|||
|
||||
## Best Practice
|
||||
|
||||
Use the `SecretText` overloads of `IsolatedStorage.SetEncrypted` and `IsolatedStorage.Get` for values that meet the definition of a secret. Check the optional Boolean result when storage failure needs a controlled error; encrypted values are subject to the documented storage-size limit. See sample: `isolatedstorage-setencrypted-for-sensitive-values.good.al`.
|
||||
Use the `SecretText` overloads of `IsolatedStorage.SetEncrypted` and `IsolatedStorage.Get` for values that meet the definition of a secret. Check the optional Boolean result when storage failure needs a controlled error; encrypted values are subject to the documented storage-size limit. See sample: [`isolatedstorage-setencrypted-for-sensitive-values.good.al`](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`.
|
||||
`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`](isolatedstorage-setencrypted-for-sensitive-values.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ application-area: [all]
|
|||
|
||||
## Best Practice
|
||||
|
||||
In SaaS, keep the value as `SecretText` and use secret-aware APIs instead of unwrapping. For an unavoidable on-premises legacy API that accepts only `Text`, keep the plain-text path as short as possible and mark every procedure in that path `[NonDebuggable]`. Do not return the unwrapped value. See sample: `nondebuggable-required-when-unwrapping-secrettext.good.al`.
|
||||
In SaaS, keep the value as `SecretText` and use secret-aware APIs instead of unwrapping. For an unavoidable on-premises legacy API that accepts only `Text`, keep the plain-text path as short as possible and mark every procedure in that path `[NonDebuggable]`. Do not return the unwrapped value. See sample: [`nondebuggable-required-when-unwrapping-secrettext.good.al`](nondebuggable-required-when-unwrapping-secrettext.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling `Unwrap()` in cloud-targeted code, or calling it in an on-premises procedure that is debuggable or returns the resulting `Text`. Both defeat the protection that `SecretText` provides. See sample: `nondebuggable-required-when-unwrapping-secrettext.bad.al`.
|
||||
Calling `Unwrap()` in cloud-targeted code, or calling it in an on-premises procedure that is debuggable or returns the resulting `Text`. Both defeat the protection that `SecretText` provides. See sample: [`nondebuggable-required-when-unwrapping-secrettext.bad.al`](nondebuggable-required-when-unwrapping-secrettext.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ A `permissionset` object can grant access object-by-object or with the `*` wildc
|
|||
|
||||
## 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`.
|
||||
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`](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`.
|
||||
`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`](permission-set-avoid-wildcard-grants.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,10 +17,10 @@ External HTTP integrations from AL can authenticate using OAuth 2.0 (client-cred
|
|||
|
||||
When the partner supports OAuth, use the platform `OAuth2` codeunit (`AcquireTokenWithClientCredentials` for service-to-service, `AcquireAuthorizationCodeTokenFromCache` for user-delegated flows) rather than hand-rolled token acquisition. Carry tokens and client secrets as `SecretText`, persist them only in IsolatedStorage, and refresh tokens proactively — on a buffer before the documented expiry — so routine calls never block on a token refresh.
|
||||
|
||||
See sample: `prefer-oauth2-over-api-keys-for-external-http-calls.good.al`.
|
||||
See sample: [`prefer-oauth2-over-api-keys-for-external-http-calls.good.al`](prefer-oauth2-over-api-keys-for-external-http-calls.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Accepting an API-key or basic-auth integration because it is the first option documented, even when the partner supports OAuth. The shared secret usually ends up in a setup-table `Text` field, rotation becomes a manual operation that rarely happens, and a single disclosure exposes every tenant using the extension.
|
||||
|
||||
See sample: `prefer-oauth2-over-api-keys-for-external-http-calls.bad.al`.
|
||||
See sample: [`prefer-oauth2-over-api-keys-for-external-http-calls.bad.al`](prefer-oauth2-over-api-keys-for-external-http-calls.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,10 +17,10 @@ A temporary record copies data out of the source table into session memory. The
|
|||
|
||||
Validate the caller's read permission on the source table before populating the temporary buffer. Keep the buffer's lifetime as short as the work requires, and prefer local temporary variables over globals for anything carrying sensitive data — a local buffer's contents are discarded automatically when the procedure returns. When a buffer must be global or is passed back to callers, delete its contents on every exit path — including error paths — so sensitive values do not linger.
|
||||
|
||||
See sample: `protect-sensitive-data-in-temporary-tables.good.al`.
|
||||
See sample: [`protect-sensitive-data-in-temporary-tables.good.al`](protect-sensitive-data-in-temporary-tables.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Copying records into a temporary buffer without a preceding permission check, and relying on procedure-exit to clean up. An exception before the explicit cleanup leaves the data in the buffer; a global or var-parameter buffer carries the data back to callers that may have no right to see it.
|
||||
|
||||
See sample: `protect-sensitive-data-in-temporary-tables.bad.al`.
|
||||
See sample: [`protect-sensitive-data-in-temporary-tables.bad.al`](protect-sensitive-data-in-temporary-tables.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ When a codeunit holds permission to system tables — directly, via a permission
|
|||
|
||||
## 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`.
|
||||
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`](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`.
|
||||
`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`](recordref-open-with-caller-table-must-not-be-public.bad.al).
|
||||
|
|
|
|||
|
|
@ -17,10 +17,10 @@ API keys, OAuth tokens, client secrets, and connection strings must not be store
|
|||
|
||||
Persist every credential in `IsolatedStorage`, write it at the point of capture, and read it only when needed. Prefer `SetEncrypted` when the value fits its documented length limit. On BC24 and later, carry the value through the `SecretText` overloads; on earlier releases, keep any required `Text` handling inside a `[NonDebuggable]` boundary. Choose the `DataScope` that matches the credential's lifetime. See `isolatedstorage-datascope-module-vs-company`, `isolatedstorage-setencrypted-for-sensitive-values`, and `secrettext-for-credentials` for those separate concerns.
|
||||
|
||||
See sample: `secrets-isolated-storage.good.al`.
|
||||
See sample: [`secrets-isolated-storage.good.al`](secrets-isolated-storage.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A "Setup" or "Connection" table carrying a `Text` field named `API Key`, `Password`, or `Client Secret`. The value is now readable by any object with table permission, ships in RapidStart packages and Excel exports, and appears in record snapshots — a credential disclosure that no amount of encryption-in-transit elsewhere makes up for. Reviewer signal: a secret-shaped field declared on a table instead of an `IsolatedStorage` call.
|
||||
|
||||
See sample: `secrets-isolated-storage.bad.al`.
|
||||
See sample: [`secrets-isolated-storage.bad.al`](secrets-isolated-storage.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ application-area: [all]
|
|||
|
||||
## Best Practice
|
||||
|
||||
Compose every secret-bearing string through `SecretStrSubstNo`, ensure the format contains a placeholder for each secret, and keep the result as `SecretText`. Pass it to `HttpRequestMessage.SetSecretRequestUri`, `HttpHeaders.Add`, or `HttpContent.WriteFrom`. See sample: `secretstrsubstno-for-composing-secrets.good.al`.
|
||||
Compose every secret-bearing string through `SecretStrSubstNo`, ensure the format contains a placeholder for each secret, and keep the result as `SecretText`. Pass it to `HttpRequestMessage.SetSecretRequestUri`, `HttpHeaders.Add`, or `HttpContent.WriteFrom`. See sample: [`secretstrsubstno-for-composing-secrets.good.al`](secretstrsubstno-for-composing-secrets.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Keeping a credential in `Text` and inserting it with `StrSubstNo`, or calling `SecretStrSubstNo` with a format that has no placeholder for the secret. The first exposes the value as plain text; the second silently omits it. See sample: `secretstrsubstno-for-composing-secrets.bad.al`.
|
||||
Keeping a credential in `Text` and inserting it with `StrSubstNo`, or calling `SecretStrSubstNo` with a format that has no placeholder for the secret. The first exposes the value as plain text; the second silently omits it. See sample: [`secretstrsubstno-for-composing-secrets.bad.al`](secretstrsubstno-for-composing-secrets.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ application-area: [all]
|
|||
|
||||
## 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 HTTP header or URI). Never round-trip through `Text`. On BC 24 and later, use the `SecretText` overload of `IsolatedStorage.Get` when retrieving stored secrets. See sample: `secrettext-for-credentials.good.al`.
|
||||
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 HTTP header or URI). Never round-trip through `Text`. On BC 24 and later, use the `SecretText` overload of `IsolatedStorage.Get` when retrieving stored secrets. See sample: [`secrettext-for-credentials.good.al`](secrettext-for-credentials.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Holding a credential in a `Text` variable (`BearerToken: Text`) makes it 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`. When the same value is visibly sent through an HTTP URI, header, or body, `secrettext-with-httpclient.md` is the more specific primary rule. See sample: `secrettext-for-credentials.bad.al`.
|
||||
Holding a credential in a `Text` variable (`BearerToken: Text`) makes it 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`. When the same value is visibly sent through an HTTP URI, header, or body, `secrettext-with-httpclient.md` is the more specific primary rule. See sample: [`secrettext-for-credentials.bad.al`](secrettext-for-credentials.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ The secret URI API belongs to `HttpRequestMessage`, not `HttpClient`. `HttpReque
|
|||
|
||||
## Best Practice
|
||||
|
||||
Compose a secret URI with `SecretStrSubstNo`, call `Request.SetSecretRequestUri(SecretUri)`, set the request method, and send the request with `HttpClient.Send(Request, Response)`. For authorization, get the request headers, add a `SecretText` value, and use `ContainsSecret` when checking for that header. See sample: `secrettext-with-httpclient.good.al`.
|
||||
Compose a secret URI with `SecretStrSubstNo`, call `Request.SetSecretRequestUri(SecretUri)`, set the request method, and send the request with `HttpClient.Send(Request, Response)`. For authorization, get the request headers, add a `SecretText` value, and use `ContainsSecret` when checking for that header. See sample: [`secrettext-with-httpclient.good.al`](secrettext-with-httpclient.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Holding a credential in `Text`, interpolating it with `StrSubstNo` or concatenation, and passing that plain text to `HttpClient.Get` or `HttpHeaders.Add`. The secret-aware request and header APIs remove the need to materialize the value as `Text`. This HTTP-sink rule supersedes the generic `secrettext-for-credentials.md` rule at the same location. See sample: `secrettext-with-httpclient.bad.al`.
|
||||
Holding a credential in `Text`, interpolating it with `StrSubstNo` or concatenation, and passing that plain text to `HttpClient.Get` or `HttpHeaders.Add`. The secret-aware request and header APIs remove the need to materialize the value as `Text`. This HTTP-sink rule supersedes the generic `secrettext-for-credentials.md` rule at the same location. See sample: [`secrettext-with-httpclient.bad.al`](secrettext-with-httpclient.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ A URL stored in a table field is user-configurable: anyone with write access to
|
|||
|
||||
## 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`.
|
||||
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`](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`.
|
||||
`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`](validate-user-configurable-urls.bad.al).
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ application-area: [all]
|
|||
|
||||
## Best Practice
|
||||
|
||||
Keep the default validation when values must exist in the related table. When free-form values are intentional, set both `ValidateTableRelation = false` and `TestTableRelation = false`, then add compensating `OnValidate` logic that normalizes, validates, creates, or otherwise handles unmatched input. Document that downstream code must not assume the relation exists. See sample: `validatetablerelation-false-on-user-input.good.al`.
|
||||
Keep the default validation when values must exist in the related table. When free-form values are intentional, set both `ValidateTableRelation = false` and `TestTableRelation = false`, then add compensating `OnValidate` logic that normalizes, validates, creates, or otherwise handles unmatched input. Document that downstream code must not assume the relation exists. See sample: [`validatetablerelation-false-on-user-input.good.al`](validatetablerelation-false-on-user-input.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`ValidateTableRelation = false` on a user-facing field with no intentional handling for unmatched values, or leaving `TestTableRelation = true` so database relation tests reject values the UI deliberately accepts. See sample: `validatetablerelation-false-on-user-input.bad.al`.
|
||||
`ValidateTableRelation = false` on a user-facing field with no intentional handling for unmatched values, or leaving `TestTableRelation = true` so database relation tests reject values the UI deliberately accepts. See sample: [`validatetablerelation-false-on-user-input.bad.al`](validatetablerelation-false-on-user-input.bad.al).
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue