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