a9f3c50863
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>
1.9 KiB
| bc-version | domain | keywords | technologies | countries | application-area | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
security |
|
|
|
|
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.