Address review feedback on security knowledge promotion

- do-not-grant-rights-beyond-a-users-entitlement.md: drop the See sample
  reference to a .good.al file that does not exist
- Remove the 'Contributions welcome' boilerplate line from
  compose-permission-sets, prefer-oauth2, and protect-sensitive-data
- protect-sensitive-data-in-temporary-tables: remove the pointless
  DeleteAll on the locally scoped temp buffer in the good sample and
  reword Best Practice to note local buffers are cleaned up automatically
- Drop guard-bulk-operations-with-istemporary from the promotion; it
  stays in the community layer pending a decision on whether it is security

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Jesper Schulz-Wedde 2026-07-10 12:27:31 +02:00
parent 347984ac74
commit 2350e10966
8 changed files with 1 additions and 12 deletions

View file

@ -0,0 +1,10 @@
codeunit 50100 "Order Buffer Helper"
{
procedure ResetStagingBuffer(var OrderBuffer: Record "Sales Header")
begin
// No IsTemporary check. A caller that accidentally passes the real
// Sales Header table wipes every sales header in the company with
// no prior warning.
OrderBuffer.DeleteAll();
end;
}

View file

@ -0,0 +1,12 @@
codeunit 50100 "Order Buffer Helper"
{
procedure ResetStagingBuffer(var OrderBuffer: Record "Sales Header")
begin
// The helper is designed for a temporary buffer only. Fail loudly
// if a caller accidentally passes the real table.
if not OrderBuffer.IsTemporary() then
Error('ResetStagingBuffer requires a temporary Sales Header; a persistent record was passed.');
OrderBuffer.DeleteAll();
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: security
keywords: [istemporary, deleteall, modifyall, safeguard, precondition]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Guard bulk operations with IsTemporary
> Contributions welcome — open a PR to refine or extend this article.
## Description
An AL helper that accepts a `var Rec: Record X` parameter and performs a bulk operation (`DeleteAll`, `ModifyAll`, or an unfiltered loop that mutates every record) cannot tell from the signature alone whether the caller passed a temporary buffer or the real table. A misuse that passes the real table wipes or rewrites live data at production scale with no earlier warning. A single `IsTemporary` check at the procedure entry turns a silent-corruption risk into an early, actionable failure.
## Best Practice
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`.
## 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`.