mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 09:26:52 +01:00
Triage seed knowledge and document admission test for preview
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.
This commit is contained in:
Jesper Schulz-Wedde
parent
5a02e6ec93
commit
23184480d0
32 changed files with 14 additions and 424 deletions
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
||||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue