From 23184480d06e9fc9e029b69d685d5717d7ef03bb Mon Sep 17 00:00:00 2001 From: Jesper Schulz-Wedde Date: Thu, 23 Apr 2026 15:47:01 +0200 Subject: [PATCH] Triage seed knowledge and document admission test for preview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove seven knowledge files whose content is generic software-engineering guidance that a capable LLM already applies without BCQuality present (HTTPS-only, secret-leakage-in-errors, no-credentials-in-URLs, silent security-error swallowing, short transaction scope, HTTP timeouts, StrSubstNo-vs-concatenation). These fail the remedial-knowledge premise and dilute the signal of the preview corpus. Strip the "Seed article — domain stewards should expand" banner from ten files that are ready to showcase (AA0232/AA0233 rules, FindSet read-only semantics, SetLoadFields ordering and usage, CalcFields-in-loops, SecretText end-to-end, DataClassification). The banner remains on files that still need domain-steward refinement. Add a "What belongs here" section to the README stating the admission test: a file exists only if a modern LLM would get something wrong or miss something without it. Gives contributors a concrete yes/no filter before they open a PR. --- README.md | 14 +++++++++ .../call-setloadfields-before-filters.md | 2 -- ...ify-every-field-with-dataclassification.md | 2 -- .../add-sift-keys-for-flowfields.md | 2 -- .../performance/avoid-calcfields-in-loops.md | 2 -- .../performance/avoid-findfirst-with-next.md | 2 -- .../performance/filter-before-find.md | 2 -- .../keep-transaction-scope-short.bad.al | 18 ------------ .../keep-transaction-scope-short.good.al | 22 -------------- .../keep-transaction-scope-short.md | 29 ------------------- .../use-findset-readonly-by-default.md | 2 -- .../use-setloadfields-for-partial-records.md | 2 -- ...e-strsubstno-for-message-formatting.bad.al | 7 ----- ...-strsubstno-for-message-formatting.good.al | 9 ------ .../use-strsubstno-for-message-formatting.md | 29 ------------------- ...id-sensitive-data-in-error-messages.bad.al | 14 --------- ...d-sensitive-data-in-error-messages.good.al | 24 --------------- .../avoid-sensitive-data-in-error-messages.md | 29 ------------------- .../do-not-put-credentials-in-urls.bad.al | 10 ------- .../do-not-put-credentials-in-urls.good.al | 13 --------- .../do-not-put-credentials-in-urls.md | 29 ------------------- ...ot-swallow-security-errors-silently.bad.al | 15 ---------- ...t-swallow-security-errors-silently.good.al | 24 --------------- ...do-not-swallow-security-errors-silently.md | 29 ------------------- .../require-https-for-external-calls.bad.al | 10 ------- .../require-https-for-external-calls.good.al | 12 -------- .../require-https-for-external-calls.md | 29 ------------------- .../set-timeouts-for-external-calls.bad.al | 11 ------- .../set-timeouts-for-external-calls.good.al | 12 -------- .../set-timeouts-for-external-calls.md | 29 ------------------- .../use-secrettext-for-credentials.md | 2 -- .../use-secrettext-with-httpclient.md | 2 -- 32 files changed, 14 insertions(+), 424 deletions(-) delete mode 100644 microsoft/knowledge/performance/keep-transaction-scope-short.bad.al delete mode 100644 microsoft/knowledge/performance/keep-transaction-scope-short.good.al delete mode 100644 microsoft/knowledge/performance/keep-transaction-scope-short.md delete mode 100644 microsoft/knowledge/performance/use-strsubstno-for-message-formatting.bad.al delete mode 100644 microsoft/knowledge/performance/use-strsubstno-for-message-formatting.good.al delete mode 100644 microsoft/knowledge/performance/use-strsubstno-for-message-formatting.md delete mode 100644 microsoft/knowledge/security/avoid-sensitive-data-in-error-messages.bad.al delete mode 100644 microsoft/knowledge/security/avoid-sensitive-data-in-error-messages.good.al delete mode 100644 microsoft/knowledge/security/avoid-sensitive-data-in-error-messages.md delete mode 100644 microsoft/knowledge/security/do-not-put-credentials-in-urls.bad.al delete mode 100644 microsoft/knowledge/security/do-not-put-credentials-in-urls.good.al delete mode 100644 microsoft/knowledge/security/do-not-put-credentials-in-urls.md delete mode 100644 microsoft/knowledge/security/do-not-swallow-security-errors-silently.bad.al delete mode 100644 microsoft/knowledge/security/do-not-swallow-security-errors-silently.good.al delete mode 100644 microsoft/knowledge/security/do-not-swallow-security-errors-silently.md delete mode 100644 microsoft/knowledge/security/require-https-for-external-calls.bad.al delete mode 100644 microsoft/knowledge/security/require-https-for-external-calls.good.al delete mode 100644 microsoft/knowledge/security/require-https-for-external-calls.md delete mode 100644 microsoft/knowledge/security/set-timeouts-for-external-calls.bad.al delete mode 100644 microsoft/knowledge/security/set-timeouts-for-external-calls.good.al delete mode 100644 microsoft/knowledge/security/set-timeouts-for-external-calls.md diff --git a/README.md b/README.md index 882e1d1..ebed31b 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,20 @@ Quality skills and knowledge for Business Central development. BCQuality is a curated knowledge base and skills library for Business Central. It provides structured, machine-readable guidance that development agents and tools can consume — establishing a consistent quality bar across tooling and teams. +## What belongs here + +BCQuality is a remedial knowledge base. A file exists because a capable LLM **would get something wrong, or miss something, without it** — not because the topic is important. The admission test for a knowledge file is one question: + +> If this file did not exist, would a modern LLM reviewing or generating BC code make a mistake this file would have prevented? + +If the answer is no — the advice is generic software-engineering guidance, or the LLM already knows the BC mechanic in question — the file does not belong here, regardless of how sound the content is. A file earns its place by encoding something BC-specific that LLMs demonstrably get wrong: a CodeCop rule number, a platform API whose semantics the training data gets backwards, a non-obvious ordering rule, a BC property whose default is a footgun. + +Good fit: "`SetLoadFields` must be called before filters, not after" (non-obvious ordering rule). "`FindSet(true)` takes a LockTable and the two-parameter signature is obsolete" (subtle platform behaviour + outdated training data). "CodeCop AA0233 flags `FindFirst … Next` loops" (rule-specific). + +Poor fit: "Use HTTPS instead of HTTP." "Don't hardcode secrets." "Keep transactions short." These are true but any capable LLM already applies them without prompting. + +The practical consequence: when a code-review agent flags something it shouldn't have, or misses something it should have caught, the remedy is a new knowledge file. When it already behaves correctly on a topic, no file is needed. + ## What's in this repo BCQuality contains **knowledge** and **skills**. It does not contain agents. Agents that consume BCQuality ship with [AL-Go](https://github.com/microsoft/AL-Go) and other orchestrators. diff --git a/community/knowledge/performance/call-setloadfields-before-filters.md b/community/knowledge/performance/call-setloadfields-before-filters.md index 90d95e8..78b455b 100644 --- a/community/knowledge/performance/call-setloadfields-before-filters.md +++ b/community/knowledge/performance/call-setloadfields-before-filters.md @@ -9,8 +9,6 @@ application-area: [all] # Call SetLoadFields before filters -> **Seed article.** Ported from BC Code Intelligence to seed the community corpus. Community contributors are invited to expand or refine. - ## Description `SetLoadFields` is folded into the database query that the subsequent `Find`, `FindSet`, or `FindFirst` executes. When it is called after filters have already been applied, the platform either ignores the specification or is forced into an extra round-trip to reload the narrower column set — negating the optimization. The placement rule is simple and absolute: `SetLoadFields` must come first. diff --git a/community/knowledge/security/classify-every-field-with-dataclassification.md b/community/knowledge/security/classify-every-field-with-dataclassification.md index 540f64b..79c27b1 100644 --- a/community/knowledge/security/classify-every-field-with-dataclassification.md +++ b/community/knowledge/security/classify-every-field-with-dataclassification.md @@ -9,8 +9,6 @@ application-area: [all] # Classify every field with DataClassification -> **Seed article.** Ported from BC Code Intelligence to seed the community corpus. Community contributors are invited to expand or refine. - ## Description Every field on every AL table and table extension must carry an explicit `DataClassification` property. The value drives GDPR tooling, data-subject requests, retention policies, and audit reporting — all of which rely on the field metadata to know what data to include, anonymize, or delete. A field with no `DataClassification` defaults to `ToBeClassified`, which is a compliance gap, not a neutral state. diff --git a/microsoft/knowledge/performance/add-sift-keys-for-flowfields.md b/microsoft/knowledge/performance/add-sift-keys-for-flowfields.md index e72eb2a..b2dce78 100644 --- a/microsoft/knowledge/performance/add-sift-keys-for-flowfields.md +++ b/microsoft/knowledge/performance/add-sift-keys-for-flowfields.md @@ -9,8 +9,6 @@ application-area: [all] # Add SIFT keys for FlowField aggregations -> **Seed article.** Converted from an existing performance-review prompt to bootstrap the BCQuality performance corpus. Domain stewards should expand, restructure, and refine as needed. - ## Description CodeCop rule AA0232 checks that FlowFields backed by CalcSums or aggregation CalcFormula are supported by a key whose SumIndexFields include the summed field and whose key prefix matches the formula's filter fields. Without a SIFT key the platform falls back to a full aggregation on every read — typically invisible in development and catastrophic in production. diff --git a/microsoft/knowledge/performance/avoid-calcfields-in-loops.md b/microsoft/knowledge/performance/avoid-calcfields-in-loops.md index 1be8cfa..c89509a 100644 --- a/microsoft/knowledge/performance/avoid-calcfields-in-loops.md +++ b/microsoft/knowledge/performance/avoid-calcfields-in-loops.md @@ -9,8 +9,6 @@ application-area: [all] # Do not call CalcFields inside loops -> **Seed article.** Converted from an existing performance-review prompt to bootstrap the BCQuality performance corpus. Domain stewards should expand, restructure, and refine as needed. - ## Description CalcFields evaluates one or more FlowFields for the current record by issuing a separate SQL aggregation. Called inside a loop over a record set, it becomes an N+1 problem: one aggregate per row. For any non-trivial set on a ledger-entry-backed FlowField this is orders of magnitude slower than the equivalent batched query. diff --git a/microsoft/knowledge/performance/avoid-findfirst-with-next.md b/microsoft/knowledge/performance/avoid-findfirst-with-next.md index bdff419..92fe53f 100644 --- a/microsoft/knowledge/performance/avoid-findfirst-with-next.md +++ b/microsoft/knowledge/performance/avoid-findfirst-with-next.md @@ -9,8 +9,6 @@ application-area: [all] # Do not pair FindFirst, FindLast, or Get with Next -> **Seed article.** Converted from an existing performance-review prompt to bootstrap the BCQuality performance corpus. Domain stewards should expand, restructure, and refine as needed. - ## Description CodeCop rule AA0233 flags loops that start with FindFirst, FindLast, or Get and then call Next. FindFirst and FindLast retrieve a single row and reposition the cursor; calling Next after them forces the platform to re-seek and stream the rest of the set, which is slower than the correct FindSet pattern and signals intent incorrectly to reviewers and the optimizer. diff --git a/microsoft/knowledge/performance/filter-before-find.md b/microsoft/knowledge/performance/filter-before-find.md index 94389b4..271a544 100644 --- a/microsoft/knowledge/performance/filter-before-find.md +++ b/microsoft/knowledge/performance/filter-before-find.md @@ -9,8 +9,6 @@ application-area: [all] # Filter before you find -> **Seed article.** Converted from an existing performance-review prompt to bootstrap the BCQuality performance corpus. Domain stewards should expand, restructure, and refine as needed. - ## Description Every call to FindSet, Find, or FindFirst on an unfiltered record variable scans the entire table. On hot tables (ledger entries, value entries, sales invoice lines) a production dataset can easily be millions of rows, so the cost of forgetting a filter is orders of magnitude worse than the cost of applying one. diff --git a/microsoft/knowledge/performance/keep-transaction-scope-short.bad.al b/microsoft/knowledge/performance/keep-transaction-scope-short.bad.al deleted file mode 100644 index 76c7972..0000000 --- a/microsoft/knowledge/performance/keep-transaction-scope-short.bad.al +++ /dev/null @@ -1,18 +0,0 @@ -codeunit 50128 "Perf Sample TxnScope Bad" -{ - procedure ImportCustomers(var Source: List of [Text]) - var - Customer: Record Customer; - HttpClient: HttpClient; - HttpResponse: HttpResponseMessage; - Row: Text; - begin - foreach Row in Source do begin - // external call inside the write transaction - HttpClient.Get('https://example.com/validate?row=' + Row, HttpResponse); - Customer.Init(); - // ... populate from Row ... - Customer.Insert(true); - end; - end; -} diff --git a/microsoft/knowledge/performance/keep-transaction-scope-short.good.al b/microsoft/knowledge/performance/keep-transaction-scope-short.good.al deleted file mode 100644 index c875f39..0000000 --- a/microsoft/knowledge/performance/keep-transaction-scope-short.good.al +++ /dev/null @@ -1,22 +0,0 @@ -codeunit 50123 "Perf Sample TxnScope Good" -{ - procedure ImportCustomers(var Source: List of [Text]) - var - Prepared: Record Customer temporary; - Customer: Record Customer; - begin - // read, validate, and shape outside the transaction - PrepareRows(Source, Prepared); - - // transaction starts here: only Insert/Modify calls - if Prepared.FindSet() then - repeat - Customer := Prepared; - Customer.Insert(true); - until Prepared.Next() = 0; - end; - - local procedure PrepareRows(var Source: List of [Text]; var Prepared: Record Customer temporary) - begin - end; -} diff --git a/microsoft/knowledge/performance/keep-transaction-scope-short.md b/microsoft/knowledge/performance/keep-transaction-scope-short.md deleted file mode 100644 index 704cffc..0000000 --- a/microsoft/knowledge/performance/keep-transaction-scope-short.md +++ /dev/null @@ -1,29 +0,0 @@ ---- -bc-version: [26..28] -domain: performance -keywords: [transaction, lock, scope, contention] -technologies: [al] -countries: [w1] -application-area: [all] ---- - -# Keep transaction scope short - -> **Seed article.** Converted from an existing performance-review prompt to bootstrap the BCQuality performance corpus. Domain stewards should expand, restructure, and refine as needed. - -## Description - -Every write operation runs inside a transaction that holds locks until the transaction ends. Long transactions increase blocking, deadlocks, and timeouts for other sessions. The same work split across narrower transactions typically completes faster under load because it holds locks for less time. - -## Best Practice - -Perform data reads, calculations, and external integrations outside the transaction whenever possible. Enter the writing phase with all inputs computed, execute the minimum set of Insert, Modify, and Delete calls, and exit. If you have a long-running batch, split it into checkpoints at safe boundaries (see avoid-commit-inside-loops). - -See sample: `keep-transaction-scope-short.good.al`. - -## Anti Pattern - -Opening a transaction, then performing external web-service calls, heavy report runs, or user-facing dialogs while the locks are held, suspends every other session that needs the same rows for as long as the external operation takes. - -See sample: `keep-transaction-scope-short.bad.al`. - diff --git a/microsoft/knowledge/performance/use-findset-readonly-by-default.md b/microsoft/knowledge/performance/use-findset-readonly-by-default.md index 64cbe72..e3d3bba 100644 --- a/microsoft/knowledge/performance/use-findset-readonly-by-default.md +++ b/microsoft/knowledge/performance/use-findset-readonly-by-default.md @@ -9,8 +9,6 @@ application-area: [all] # Use FindSet in read-only mode by default -> **Seed article.** Converted from an existing performance-review prompt to bootstrap the BCQuality performance corpus. Domain stewards should expand, restructure, and refine as needed. - ## Description FindSet has two modes: FindSet() and FindSet(false) are read-only and take no write lock; FindSet(true) calls LockTable before fetching. Write locks are expensive and hold for the remainder of the transaction, so passing `true` when you do not intend to modify the records increases contention under load. diff --git a/microsoft/knowledge/performance/use-setloadfields-for-partial-records.md b/microsoft/knowledge/performance/use-setloadfields-for-partial-records.md index e112fc7..bcbf064 100644 --- a/microsoft/knowledge/performance/use-setloadfields-for-partial-records.md +++ b/microsoft/knowledge/performance/use-setloadfields-for-partial-records.md @@ -9,8 +9,6 @@ application-area: [all] # Use SetLoadFields for partial records -> **Seed article.** Converted from an existing performance-review prompt to bootstrap the BCQuality performance corpus. Domain stewards should expand, restructure, and refine as needed. - ## Description SetLoadFields instructs the platform to hydrate only the listed fields on a record variable. On wide tables, or tables with BLOB or media fields, the difference is substantial: a Sales Invoice Line has dozens of fields and loading all of them for every row of a large set is wasted bandwidth. Primary key fields, SystemId, and system audit fields are always loaded automatically. SetLoadFields works only with FieldClass = Normal; FlowFields and FlowFilters cannot be partial-loaded. diff --git a/microsoft/knowledge/performance/use-strsubstno-for-message-formatting.bad.al b/microsoft/knowledge/performance/use-strsubstno-for-message-formatting.bad.al deleted file mode 100644 index bcfa7d0..0000000 --- a/microsoft/knowledge/performance/use-strsubstno-for-message-formatting.bad.al +++ /dev/null @@ -1,7 +0,0 @@ -codeunit 50137 "Perf Sample StrSubstNo Bad" -{ - procedure CustomerGreeting(var Customer: Record Customer): Text - begin - exit('Hello, ' + Customer.Name + ' (' + Customer."No." + ')'); - end; -} diff --git a/microsoft/knowledge/performance/use-strsubstno-for-message-formatting.good.al b/microsoft/knowledge/performance/use-strsubstno-for-message-formatting.good.al deleted file mode 100644 index 87883e8..0000000 --- a/microsoft/knowledge/performance/use-strsubstno-for-message-formatting.good.al +++ /dev/null @@ -1,9 +0,0 @@ -codeunit 50136 "Perf Sample StrSubstNo Good" -{ - procedure CustomerGreeting(var Customer: Record Customer): Text - var - GreetingLbl: Label 'Hello, %1 (%2)'; - begin - exit(StrSubstNo(GreetingLbl, Customer.Name, Customer."No.")); - end; -} diff --git a/microsoft/knowledge/performance/use-strsubstno-for-message-formatting.md b/microsoft/knowledge/performance/use-strsubstno-for-message-formatting.md deleted file mode 100644 index 6e2b3f6..0000000 --- a/microsoft/knowledge/performance/use-strsubstno-for-message-formatting.md +++ /dev/null @@ -1,29 +0,0 @@ ---- -bc-version: [26..28] -domain: performance -keywords: [strsubstno, string, concatenation, format] -technologies: [al] -countries: [w1] -application-area: [all] ---- - -# Use StrSubstNo for message formatting - -> **Seed article.** Converted from an existing performance-review prompt to bootstrap the BCQuality performance corpus. Domain stewards should expand, restructure, and refine as needed. - -## Description - -StrSubstNo formats values into a placeholder template in a single call. Manual concatenation with `+` produces a chain of intermediate strings, each allocated and discarded, and mixes formatting rules inconsistently across locales. The performance difference per call is small; repeated inside a tight loop it is noticeable. - -## Best Practice - -Declare the template as a Label (so it can be localized) and format with StrSubstNo. Pass values in the order the placeholders expect; StrSubstNo handles locale-sensitive conversions consistently. - -See sample: `use-strsubstno-for-message-formatting.good.al`. - -## Anti Pattern - -Building a user-facing string by concatenating record field values with string literals ignores locale rules and allocates more than necessary. - -See sample: `use-strsubstno-for-message-formatting.bad.al`. - diff --git a/microsoft/knowledge/security/avoid-sensitive-data-in-error-messages.bad.al b/microsoft/knowledge/security/avoid-sensitive-data-in-error-messages.bad.al deleted file mode 100644 index 0a78e75..0000000 --- a/microsoft/knowledge/security/avoid-sensitive-data-in-error-messages.bad.al +++ /dev/null @@ -1,14 +0,0 @@ -codeunit 50225 "Sec Sample ErrorDisclosure Bad" -{ - procedure Connect() - begin - if not TryConnect() then - Error('Failed to connect to Server=PROD-SQL01;Database=NAV;User=svc_admin: %1', GetLastErrorText()); - end; - - [TryFunction] - local procedure TryConnect() - begin - // ... - end; -} diff --git a/microsoft/knowledge/security/avoid-sensitive-data-in-error-messages.good.al b/microsoft/knowledge/security/avoid-sensitive-data-in-error-messages.good.al deleted file mode 100644 index 6fb6267..0000000 --- a/microsoft/knowledge/security/avoid-sensitive-data-in-error-messages.good.al +++ /dev/null @@ -1,24 +0,0 @@ -codeunit 50224 "Sec Sample ErrorDisclosure Good" -{ - var - ConnectionFailedErr: Label 'Connection to the external service failed. Contact your administrator.'; - - procedure Connect() - begin - if not TryConnect() then begin - LogConnectionFailure(GetLastErrorText()); - Error(ConnectionFailedErr); - end; - end; - - [TryFunction] - local procedure TryConnect() - begin - // ... - end; - - local procedure LogConnectionFailure(Detail: Text) - begin - // Route to controlled logging (Session.LogMessage, activity log, etc.). - end; -} diff --git a/microsoft/knowledge/security/avoid-sensitive-data-in-error-messages.md b/microsoft/knowledge/security/avoid-sensitive-data-in-error-messages.md deleted file mode 100644 index 75d78cb..0000000 --- a/microsoft/knowledge/security/avoid-sensitive-data-in-error-messages.md +++ /dev/null @@ -1,29 +0,0 @@ ---- -bc-version: [26..28] -domain: security -keywords: [error, disclosure, logging, label] -technologies: [al] -countries: [w1] -application-area: [all] ---- - -# Avoid sensitive data in error messages - -> **Seed article.** Converted from an existing security-review prompt to bootstrap the BCQuality security corpus. Domain stewards should expand, restructure, and refine as needed. - -## Description - -Errors surfaced to end users are routinely forwarded to support systems, captured in bug reports, and exported to telemetry. Server names, database names, usernames, connection strings, file paths, and stack excerpts in an end-user error message leak infrastructure detail to untrusted consumers and help an attacker map the environment. - -## Best Practice - -Raise end-user errors using localized Labels that describe the condition without naming infrastructure. Emit the actual detail (exception text, endpoint, correlation id) through the application's internal logging channel, where audience and retention are controlled. - -See sample: `avoid-sensitive-data-in-error-messages.good.al`. - -## Anti Pattern - -Error('Failed to connect to Server=PROD-SQL01;Database=NAV;User=admin: %1', Ex.Message); — every support ticket now carries the server name, database name, and service account. - -See sample: `avoid-sensitive-data-in-error-messages.bad.al`. - diff --git a/microsoft/knowledge/security/do-not-put-credentials-in-urls.bad.al b/microsoft/knowledge/security/do-not-put-credentials-in-urls.bad.al deleted file mode 100644 index 12476bb..0000000 --- a/microsoft/knowledge/security/do-not-put-credentials-in-urls.bad.al +++ /dev/null @@ -1,10 +0,0 @@ -codeunit 50223 "Sec Sample UrlCreds Bad" -{ - procedure Call(ApiKey: Text) - var - Client: HttpClient; - Response: HttpResponseMessage; - begin - Client.Get('https://api.example.com/v1/items?api_key=' + ApiKey, Response); - end; -} diff --git a/microsoft/knowledge/security/do-not-put-credentials-in-urls.good.al b/microsoft/knowledge/security/do-not-put-credentials-in-urls.good.al deleted file mode 100644 index c26dd29..0000000 --- a/microsoft/knowledge/security/do-not-put-credentials-in-urls.good.al +++ /dev/null @@ -1,13 +0,0 @@ -codeunit 50222 "Sec Sample UrlCreds Good" -{ - procedure Call(ApiKey: SecretText) - var - Client: HttpClient; - Response: HttpResponseMessage; - AuthHeader: SecretText; - begin - AuthHeader := SecretStrSubstNo('Bearer %1', ApiKey); - Client.DefaultRequestHeaders.Add('Authorization', AuthHeader); - Client.Get('https://api.example.com/v1/items', Response); - end; -} diff --git a/microsoft/knowledge/security/do-not-put-credentials-in-urls.md b/microsoft/knowledge/security/do-not-put-credentials-in-urls.md deleted file mode 100644 index dc3843e..0000000 --- a/microsoft/knowledge/security/do-not-put-credentials-in-urls.md +++ /dev/null @@ -1,29 +0,0 @@ ---- -bc-version: [26..28] -domain: security -keywords: [url, query-string, credentials, logging] -technologies: [al] -countries: [w1] -application-area: [all] ---- - -# Do not put credentials in URLs - -> **Seed article.** Converted from an existing security-review prompt to bootstrap the BCQuality security corpus. Domain stewards should expand, restructure, and refine as needed. - -## Description - -URL query strings and path segments are routinely captured in web-server access logs, browser history, proxy logs, platform telemetry, and exception traces. A credential placed anywhere in the URL therefore persists across systems the extension does not control, and is typically retained far longer than the secret's intended lifetime. - -## Best Practice - -Transport credentials in Authorization headers, carried as SecretText end-to-end (see use-secrettext-with-httpclient). Where the URI itself must carry a secret (for example, a pre-signed URL), build it with SecretStrSubstNo and pass it via SetSecretRequestUri so it is never materialized as Text. - -See sample: `do-not-put-credentials-in-urls.good.al`. - -## Anti Pattern - -Appending '?api_key=' + Key to a request URL, or embedding a token in a path segment, then calling HttpClient.Get with the resulting Text URL. - -See sample: `do-not-put-credentials-in-urls.bad.al`. - diff --git a/microsoft/knowledge/security/do-not-swallow-security-errors-silently.bad.al b/microsoft/knowledge/security/do-not-swallow-security-errors-silently.bad.al deleted file mode 100644 index 923df57..0000000 --- a/microsoft/knowledge/security/do-not-swallow-security-errors-silently.bad.al +++ /dev/null @@ -1,15 +0,0 @@ -codeunit 50227 "Sec Sample SwallowErr Bad" -{ - procedure Authenticate(): Boolean - begin - if not TryAuthenticate() then - exit(false); - exit(true); - end; - - [TryFunction] - local procedure TryAuthenticate() - begin - // ... - end; -} diff --git a/microsoft/knowledge/security/do-not-swallow-security-errors-silently.good.al b/microsoft/knowledge/security/do-not-swallow-security-errors-silently.good.al deleted file mode 100644 index 2887312..0000000 --- a/microsoft/knowledge/security/do-not-swallow-security-errors-silently.good.al +++ /dev/null @@ -1,24 +0,0 @@ -codeunit 50226 "Sec Sample SwallowErr Good" -{ - procedure Authenticate(): Boolean - begin - if TryAuthenticate() then - exit(true); - - LogAuthFailure(GetLastErrorText()); - exit(false); - end; - - [TryFunction] - local procedure TryAuthenticate() - begin - // ... - end; - - local procedure LogAuthFailure(Detail: Text) - begin - Session.LogMessage('SEC0001', 'Authentication failed', Verbosity::Warning, - DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher, - 'Detail', Detail); - end; -} diff --git a/microsoft/knowledge/security/do-not-swallow-security-errors-silently.md b/microsoft/knowledge/security/do-not-swallow-security-errors-silently.md deleted file mode 100644 index fa4af42..0000000 --- a/microsoft/knowledge/security/do-not-swallow-security-errors-silently.md +++ /dev/null @@ -1,29 +0,0 @@ ---- -bc-version: [26..28] -domain: security -keywords: [tryfunction, logging, audit, error] -technologies: [al] -countries: [w1] -application-area: [all] ---- - -# Do not swallow security errors silently - -> **Seed article.** Converted from an existing security-review prompt to bootstrap the BCQuality security corpus. Domain stewards should expand, restructure, and refine as needed. - -## Description - -Authentication failures, permission denials, and unexpected error paths in security-relevant code are the signals a reviewer or incident responder needs to see. A TryFunction whose failure is ignored without logging turns an attack or a misconfiguration into silent bad behaviour: the call returns false, the caller moves on, and no record of the event survives. - -## Best Practice - -Use TryFunctions to contain errors around security-relevant work, but always log the failure (category, GetLastErrorText, and enough context to identify the operation) before deciding whether to surface a user-facing error. Never discard a caught security error without a trace. - -See sample: `do-not-swallow-security-errors-silently.good.al`. - -## Anti Pattern - -`if not TryAuthenticate() then exit;` with no logging and no user-facing error. An authentication-bypass attempt, a revoked credential, and a transient network glitch are now indistinguishable. - -See sample: `do-not-swallow-security-errors-silently.bad.al`. - diff --git a/microsoft/knowledge/security/require-https-for-external-calls.bad.al b/microsoft/knowledge/security/require-https-for-external-calls.bad.al deleted file mode 100644 index 4edf291..0000000 --- a/microsoft/knowledge/security/require-https-for-external-calls.bad.al +++ /dev/null @@ -1,10 +0,0 @@ -codeunit 50219 "Sec Sample Https Bad" -{ - procedure CallExternal() - var - Client: HttpClient; - Response: HttpResponseMessage; - begin - Client.Get('http://api.example.com/data', Response); - end; -} diff --git a/microsoft/knowledge/security/require-https-for-external-calls.good.al b/microsoft/knowledge/security/require-https-for-external-calls.good.al deleted file mode 100644 index 6ae7639..0000000 --- a/microsoft/knowledge/security/require-https-for-external-calls.good.al +++ /dev/null @@ -1,12 +0,0 @@ -codeunit 50218 "Sec Sample Https Good" -{ - procedure CallExternal(Endpoint: Text) - var - Client: HttpClient; - Response: HttpResponseMessage; - begin - if not Endpoint.StartsWith('https://') then - Error('Only HTTPS endpoints are allowed.'); - Client.Get(Endpoint, Response); - end; -} diff --git a/microsoft/knowledge/security/require-https-for-external-calls.md b/microsoft/knowledge/security/require-https-for-external-calls.md deleted file mode 100644 index a026015..0000000 --- a/microsoft/knowledge/security/require-https-for-external-calls.md +++ /dev/null @@ -1,29 +0,0 @@ ---- -bc-version: [26..28] -domain: security -keywords: [https, httpclient, tls, plaintext] -technologies: [al] -countries: [w1] -application-area: [all] ---- - -# Require HTTPS for external calls - -> **Seed article.** Converted from an existing security-review prompt to bootstrap the BCQuality security corpus. Domain stewards should expand, restructure, and refine as needed. - -## Description - -HttpClient can issue requests over plaintext HTTP as easily as over HTTPS. A request sent over http:// is transmitted unencrypted, exposing the full URL (including query string), the request headers (including Authorization), and the bodies of both request and response to any on-path observer. This holds even when the payload itself is not marked sensitive — request signatures and session tokens are routinely captured and replayed. - -## Best Practice - -Call external services exclusively over https://. When the destination is configurable, validate at runtime that the scheme is https before issuing the request, and fail closed with a clear (non-disclosing) error otherwise. - -See sample: `require-https-for-external-calls.good.al`. - -## Anti Pattern - -Issuing HttpClient.Get('http://...'), or accepting an arbitrary user-supplied URL and passing it straight to HttpClient without scheme validation. - -See sample: `require-https-for-external-calls.bad.al`. - diff --git a/microsoft/knowledge/security/set-timeouts-for-external-calls.bad.al b/microsoft/knowledge/security/set-timeouts-for-external-calls.bad.al deleted file mode 100644 index 2068c1f..0000000 --- a/microsoft/knowledge/security/set-timeouts-for-external-calls.bad.al +++ /dev/null @@ -1,11 +0,0 @@ -codeunit 50221 "Sec Sample Timeout Bad" -{ - procedure CallExternal() - var - Client: HttpClient; - Response: HttpResponseMessage; - begin - // No Timeout set; a hung endpoint stalls the caller. - Client.Get('https://api.example.com/data', Response); - end; -} diff --git a/microsoft/knowledge/security/set-timeouts-for-external-calls.good.al b/microsoft/knowledge/security/set-timeouts-for-external-calls.good.al deleted file mode 100644 index ad9910c..0000000 --- a/microsoft/knowledge/security/set-timeouts-for-external-calls.good.al +++ /dev/null @@ -1,12 +0,0 @@ -codeunit 50220 "Sec Sample Timeout Good" -{ - procedure CallExternal() - var - Client: HttpClient; - Response: HttpResponseMessage; - begin - Client.Timeout := 10000; // 10 seconds - if not Client.Get('https://api.example.com/data', Response) then - Error('External service is unavailable.'); - end; -} diff --git a/microsoft/knowledge/security/set-timeouts-for-external-calls.md b/microsoft/knowledge/security/set-timeouts-for-external-calls.md deleted file mode 100644 index 28d83b0..0000000 --- a/microsoft/knowledge/security/set-timeouts-for-external-calls.md +++ /dev/null @@ -1,29 +0,0 @@ ---- -bc-version: [26..28] -domain: security -keywords: [timeout, httpclient, availability, dos] -technologies: [al] -countries: [w1] -application-area: [all] ---- - -# Set timeouts for external calls - -> **Seed article.** Converted from an existing security-review prompt to bootstrap the BCQuality security corpus. Domain stewards should expand, restructure, and refine as needed. - -## Description - -An HttpClient with no explicit timeout relies on defaults that may be long enough for a hung or slow endpoint to block a user session or a background task for minutes. A dependency that degrades therefore degrades the caller, and an intentionally slow endpoint is a cheap denial-of-service vector against the extension. - -## Best Practice - -Set HttpClient.Timeout to a bounded value (seconds, not minutes) that reflects the SLA of the dependency. Handle the timeout error without leaking endpoint details to end users (see avoid-sensitive-data-in-error-messages). - -See sample: `set-timeouts-for-external-calls.good.al`. - -## Anti Pattern - -Issuing HttpClient requests without setting Timeout and without a timeout-handling branch. A slow dependency now has an unbounded blast radius inside the extension. - -See sample: `set-timeouts-for-external-calls.bad.al`. - diff --git a/microsoft/knowledge/security/use-secrettext-for-credentials.md b/microsoft/knowledge/security/use-secrettext-for-credentials.md index 991f2d0..0401d9a 100644 --- a/microsoft/knowledge/security/use-secrettext-for-credentials.md +++ b/microsoft/knowledge/security/use-secrettext-for-credentials.md @@ -9,8 +9,6 @@ application-area: [all] # Use SecretText for credentials -> **Seed article.** Converted from an existing security-review prompt to bootstrap the BCQuality security corpus. Domain stewards should expand, restructure, and refine as needed. - ## 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. diff --git a/microsoft/knowledge/security/use-secrettext-with-httpclient.md b/microsoft/knowledge/security/use-secrettext-with-httpclient.md index 2acb268..6a0ed9a 100644 --- a/microsoft/knowledge/security/use-secrettext-with-httpclient.md +++ b/microsoft/knowledge/security/use-secrettext-with-httpclient.md @@ -9,8 +9,6 @@ application-area: [all] # Use SecretText with HttpClient -> **Seed article.** Converted from an existing security-review prompt to bootstrap the BCQuality security corpus. Domain stewards should expand, restructure, and refine as needed. - ## 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.