mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-07 01:46:53 +01:00
Seed security knowledge corpus (16 articles + 30 AL samples)
Converts Jesper's existing AL security-review prompt into BCQuality seed
knowledge articles so the al-security-review leaf has a real corpus to
match against. Mirrors the performance seed phase.
Articles under microsoft/knowledge/security/ (16):
- Permission model: follow-least-privilege-in-permission-sets,
use-indirect-permissions-for-elevated-access,
use-inherent-permissions-to-grant-minimal-access
- Secrets: never-hardcode-secrets-in-al,
use-isolated-storage-for-module-and-company-secrets,
prefer-azure-key-vault-for-production-secrets,
use-secrettext-for-credentials, use-secrettext-with-httpclient,
compose-secrets-with-secretstrsubstno,
use-nondebuggable-when-parsing-secrets
- External calls: require-https-for-external-calls,
set-timeouts-for-external-calls, do-not-put-credentials-in-urls
- Error handling: avoid-sensitive-data-in-error-messages,
do-not-swallow-security-errors-silently
- Extensibility: do-not-expose-sensitive-data-in-event-publishers
Paired AL samples under samples/security/<slug>/{bad,good}.al, object
IDs 50200-50231 (no overlap with performance 50100-50140).
Rubber-duck findings addressed:
- HttpClient secret-URI: SetSecretRequestUri is on HttpRequestMessage
(not HttpClient). Rewrote use-secrettext-with-httpclient and its
good sample to use HttpRequestMessage + HttpClient.Send.
- InherentPermissions only grants access to same-extension objects;
the sample now defines its own table 50230 "Sec Sample Lookup" and
grants 'r' on that, not on Database::Customer.
- Reworked compose-secrets-with-secretstrsubstno bad.al away from
Format(SecretText) (unreliable) to a plain Text+StrSubstNo anti-
pattern.
- Moved normative guidance out of Description in three articles
(compose-secrets-..., prefer-azure-key-vault-..., use-inherent-...)
so it sits in Best Practice / Anti Pattern per READ contract.
- Added a companion helper codeunit (50231) to the indirect-permissions
good sample so it actually demonstrates the controlled write path.
- Rebuilt the event-publisher good/bad pair on the same ExportCustomer
scenario so the contrast is the shape of the event signature, not a
different event.
Also: broaden samples/README.md object-ID range note to 50100-50299.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
parent
32c40bbf1d
commit
0980397d27
47 changed files with 899 additions and 1 deletions
|
|
@ -0,0 +1,14 @@
|
|||
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;
|
||||
}
|
||||
|
|
@ -0,0 +1,24 @@
|
|||
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;
|
||||
}
|
||||
|
|
@ -0,0 +1,9 @@
|
|||
codeunit 50215 "Sec Sample SecretCompose Bad"
|
||||
{
|
||||
procedure BuildAuthHeader(Token: Text) AuthHeader: Text
|
||||
begin
|
||||
// Token is Text, so the combined value is plaintext.
|
||||
// The whole shape should have used SecretText + SecretStrSubstNo.
|
||||
AuthHeader := StrSubstNo('Bearer %1', Token);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,7 @@
|
|||
codeunit 50214 "Sec Sample SecretCompose Good"
|
||||
{
|
||||
procedure BuildAuthHeader(Token: SecretText) AuthHeader: SecretText
|
||||
begin
|
||||
AuthHeader := SecretStrSubstNo('Bearer %1', Token);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,19 @@
|
|||
codeunit 50229 "Sec Sample EventPublisher Bad"
|
||||
{
|
||||
[IntegrationEvent(false, false)]
|
||||
local procedure OnBeforeExportCustomer(CustomerNo: Code[20]; ExportCredentials: SecretText; var AllowExport: Boolean)
|
||||
begin
|
||||
end;
|
||||
|
||||
procedure ExportCustomer(CustomerNo: Code[20]; Credentials: SecretText)
|
||||
var
|
||||
AllowExport: Boolean;
|
||||
begin
|
||||
// Any subscriber on the tenant receives the credentials and
|
||||
// can flip AllowExport := true to bypass the publisher's check.
|
||||
OnBeforeExportCustomer(CustomerNo, Credentials, AllowExport);
|
||||
if not AllowExport then
|
||||
exit;
|
||||
// ... perform export
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,23 @@
|
|||
codeunit 50228 "Sec Sample EventPublisher Good"
|
||||
{
|
||||
[IntegrationEvent(false, false)]
|
||||
local procedure OnBeforeExportCustomer(CustomerNo: Code[20])
|
||||
begin
|
||||
end;
|
||||
|
||||
procedure ExportCustomer(CustomerNo: Code[20])
|
||||
begin
|
||||
if not CallerIsAuthorizedToExport(CustomerNo) then
|
||||
Error('You are not authorized to export this customer.');
|
||||
|
||||
OnBeforeExportCustomer(CustomerNo);
|
||||
// ... perform export using credentials owned by this codeunit
|
||||
end;
|
||||
|
||||
local procedure CallerIsAuthorizedToExport(CustomerNo: Code[20]): Boolean
|
||||
begin
|
||||
// Authorization decision stays inside the publisher. Subscribers
|
||||
// receive only the customer number and cannot influence the
|
||||
// decision.
|
||||
end;
|
||||
}
|
||||
10
samples/security/do-not-put-credentials-in-urls/bad.al
Normal file
10
samples/security/do-not-put-credentials-in-urls/bad.al
Normal file
|
|
@ -0,0 +1,10 @@
|
|||
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;
|
||||
}
|
||||
13
samples/security/do-not-put-credentials-in-urls/good.al
Normal file
13
samples/security/do-not-put-credentials-in-urls/good.al
Normal file
|
|
@ -0,0 +1,13 @@
|
|||
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;
|
||||
}
|
||||
|
|
@ -0,0 +1,15 @@
|
|||
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;
|
||||
}
|
||||
|
|
@ -0,0 +1,24 @@
|
|||
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;
|
||||
}
|
||||
|
|
@ -0,0 +1,6 @@
|
|||
permissionset 50201 "Sec Sample Full Access"
|
||||
{
|
||||
Assignable = true;
|
||||
Caption = 'Full Access (sample anti-pattern)';
|
||||
Permissions = tabledata * = RIMD;
|
||||
}
|
||||
|
|
@ -0,0 +1,9 @@
|
|||
permissionset 50200 "Sec Sample Sales Order Entry"
|
||||
{
|
||||
Assignable = true;
|
||||
Caption = 'Sales Order Entry (sample)';
|
||||
Permissions =
|
||||
tabledata "Sales Header" = RIM,
|
||||
tabledata "Sales Line" = RIMD,
|
||||
tabledata Customer = R;
|
||||
}
|
||||
10
samples/security/never-hardcode-secrets-in-al/bad.al
Normal file
10
samples/security/never-hardcode-secrets-in-al/bad.al
Normal file
|
|
@ -0,0 +1,10 @@
|
|||
codeunit 50207 "Sec Sample HardcodedSecret Bad"
|
||||
{
|
||||
var
|
||||
HardcodedApiKeyLbl: Label 'sk-live-1234567890abcdef', Locked = true;
|
||||
|
||||
procedure GetApiKey(): Text
|
||||
begin
|
||||
exit(HardcodedApiKeyLbl);
|
||||
end;
|
||||
}
|
||||
12
samples/security/never-hardcode-secrets-in-al/good.al
Normal file
12
samples/security/never-hardcode-secrets-in-al/good.al
Normal file
|
|
@ -0,0 +1,12 @@
|
|||
codeunit 50206 "Sec Sample HardcodedSecret Good"
|
||||
{
|
||||
procedure GetApiKey() ApiKey: SecretText
|
||||
var
|
||||
StoredValue: SecretText;
|
||||
begin
|
||||
if IsolatedStorage.Contains('ApiKey', DataScope::Module) then
|
||||
if IsolatedStorage.Get('ApiKey', DataScope::Module, StoredValue) then
|
||||
exit(StoredValue);
|
||||
Error('API key is not configured.');
|
||||
end;
|
||||
}
|
||||
10
samples/security/require-https-for-external-calls/bad.al
Normal file
10
samples/security/require-https-for-external-calls/bad.al
Normal file
|
|
@ -0,0 +1,10 @@
|
|||
codeunit 50219 "Sec Sample Https Bad"
|
||||
{
|
||||
procedure CallExternal()
|
||||
var
|
||||
Client: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
begin
|
||||
Client.Get('http://api.example.com/data', Response);
|
||||
end;
|
||||
}
|
||||
12
samples/security/require-https-for-external-calls/good.al
Normal file
12
samples/security/require-https-for-external-calls/good.al
Normal file
|
|
@ -0,0 +1,12 @@
|
|||
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;
|
||||
}
|
||||
11
samples/security/set-timeouts-for-external-calls/bad.al
Normal file
11
samples/security/set-timeouts-for-external-calls/bad.al
Normal file
|
|
@ -0,0 +1,11 @@
|
|||
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;
|
||||
}
|
||||
12
samples/security/set-timeouts-for-external-calls/good.al
Normal file
12
samples/security/set-timeouts-for-external-calls/good.al
Normal file
|
|
@ -0,0 +1,12 @@
|
|||
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;
|
||||
}
|
||||
|
|
@ -0,0 +1,7 @@
|
|||
permissionset 50203 "Sec Sample Direct Write"
|
||||
{
|
||||
Assignable = true;
|
||||
Caption = 'Direct write granted to every caller (sample anti-pattern)';
|
||||
Permissions =
|
||||
tabledata "Sales Header" = RM;
|
||||
}
|
||||
|
|
@ -0,0 +1,34 @@
|
|||
permissionset 50202 "Sec Sample Elevated Write"
|
||||
{
|
||||
Assignable = false;
|
||||
Caption = 'Elevated write via helper (sample)';
|
||||
// Callers hold R directly; the helper codeunit assumes this set and
|
||||
// performs the Modify via indirect permission.
|
||||
Permissions =
|
||||
tabledata "Sales Header" = Rmi;
|
||||
}
|
||||
|
||||
codeunit 50231 "Sec Sample Elevated Helper"
|
||||
{
|
||||
Access = Public;
|
||||
Permissions = tabledata "Sales Header" = Rmi;
|
||||
|
||||
procedure SetExternalDocumentNo(SalesDocType: Enum "Sales Document Type"; SalesDocNo: Code[20]; NewExternalDocNo: Code[35])
|
||||
var
|
||||
SalesHeader: Record "Sales Header";
|
||||
begin
|
||||
ValidateCaller();
|
||||
if NewExternalDocNo = '' then
|
||||
Error('External document number must be provided.');
|
||||
if not SalesHeader.Get(SalesDocType, SalesDocNo) then
|
||||
Error('Sales document not found.');
|
||||
SalesHeader."External Document No." := NewExternalDocNo;
|
||||
SalesHeader.Modify(true);
|
||||
end;
|
||||
|
||||
local procedure ValidateCaller()
|
||||
begin
|
||||
// Verify the caller is permitted to perform this elevated write
|
||||
// (role check, setup flag, approvals, etc.).
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,13 @@
|
|||
codeunit 50205 "Sec Sample Inherent Bad"
|
||||
{
|
||||
// No InherentPermissions attribute: every caller must hold
|
||||
// tabledata "Sec Sample Lookup" = R just to look up a name.
|
||||
procedure GetLookupName(LookupCode: Code[20]): Text[100]
|
||||
var
|
||||
Lookup: Record "Sec Sample Lookup";
|
||||
begin
|
||||
if Lookup.Get(LookupCode) then
|
||||
exit(Lookup.Name);
|
||||
exit('');
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
table 50230 "Sec Sample Lookup"
|
||||
{
|
||||
DataClassification = SystemMetadata;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Code"; Code[20]) { }
|
||||
field(2; "Name"; Text[100]) { }
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Code") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50204 "Sec Sample Inherent Good"
|
||||
{
|
||||
[InherentPermissions(PermissionObjectType::TableData, Database::"Sec Sample Lookup", 'r')]
|
||||
procedure GetLookupName(LookupCode: Code[20]): Text[100]
|
||||
var
|
||||
Lookup: Record "Sec Sample Lookup";
|
||||
begin
|
||||
if Lookup.Get(LookupCode) then
|
||||
exit(Lookup.Name);
|
||||
exit('');
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,17 @@
|
|||
codeunit 50209 "Sec Sample IsolatedStorage Bad"
|
||||
{
|
||||
procedure StoreApiKey(NewKey: Text)
|
||||
begin
|
||||
// Plaintext write to IsolatedStorage is not encrypted at rest.
|
||||
IsolatedStorage.Set('ApiKey', NewKey, DataScope::Module);
|
||||
end;
|
||||
|
||||
procedure GetApiKey(): Text
|
||||
var
|
||||
ApiKey: Text;
|
||||
begin
|
||||
if IsolatedStorage.Get('ApiKey', DataScope::Module, ApiKey) then
|
||||
exit(ApiKey);
|
||||
exit('');
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,14 @@
|
|||
codeunit 50208 "Sec Sample IsolatedStorage Good"
|
||||
{
|
||||
procedure StoreApiKey(NewKey: SecretText)
|
||||
begin
|
||||
IsolatedStorage.SetEncrypted('ApiKey', NewKey, DataScope::Module);
|
||||
end;
|
||||
|
||||
procedure TryGetApiKey(var ApiKey: SecretText): Boolean
|
||||
begin
|
||||
if IsolatedStorage.Contains('ApiKey', DataScope::Module) then
|
||||
exit(IsolatedStorage.Get('ApiKey', DataScope::Module, ApiKey));
|
||||
exit(false);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,16 @@
|
|||
codeunit 50217 "Sec Sample NonDebuggable Bad"
|
||||
{
|
||||
// Missing [NonDebuggable]: ResponseText and the extracted token are
|
||||
// inspectable in the debugger and in snapshot debug sessions.
|
||||
procedure ParseSessionToken(Response: HttpResponseMessage; var SessionToken: SecretText)
|
||||
var
|
||||
ResponseText: Text;
|
||||
JObject: JsonObject;
|
||||
JToken: JsonToken;
|
||||
begin
|
||||
Response.Content.ReadAs(ResponseText);
|
||||
JObject.ReadFrom(ResponseText);
|
||||
JObject.Get('access_token', JToken);
|
||||
SessionToken := JToken.AsValue().AsText();
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,15 @@
|
|||
codeunit 50216 "Sec Sample NonDebuggable Good"
|
||||
{
|
||||
[NonDebuggable]
|
||||
procedure ParseSessionToken(Response: HttpResponseMessage; var SessionToken: SecretText)
|
||||
var
|
||||
ResponseText: Text;
|
||||
JObject: JsonObject;
|
||||
JToken: JsonToken;
|
||||
begin
|
||||
Response.Content.ReadAs(ResponseText);
|
||||
JObject.ReadFrom(ResponseText);
|
||||
JObject.Get('access_token', JToken);
|
||||
SessionToken := JToken.AsValue().AsText();
|
||||
end;
|
||||
}
|
||||
13
samples/security/use-secrettext-for-credentials/bad.al
Normal file
13
samples/security/use-secrettext-for-credentials/bad.al
Normal file
|
|
@ -0,0 +1,13 @@
|
|||
codeunit 50211 "Sec Sample SecretText Bad"
|
||||
{
|
||||
procedure SendAuthenticatedRequest(BearerToken: Text)
|
||||
var
|
||||
Client: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
AuthValue: Text;
|
||||
begin
|
||||
AuthValue := 'Bearer ' + BearerToken;
|
||||
Client.DefaultRequestHeaders.Add('Authorization', AuthValue);
|
||||
Client.Get('https://api.example.com/data', Response);
|
||||
end;
|
||||
}
|
||||
14
samples/security/use-secrettext-for-credentials/good.al
Normal file
14
samples/security/use-secrettext-for-credentials/good.al
Normal file
|
|
@ -0,0 +1,14 @@
|
|||
codeunit 50210 "Sec Sample SecretText Good"
|
||||
{
|
||||
procedure SendAuthenticatedRequest(BearerToken: SecretText)
|
||||
var
|
||||
Client: HttpClient;
|
||||
Headers: HttpHeaders;
|
||||
Response: HttpResponseMessage;
|
||||
AuthValue: SecretText;
|
||||
begin
|
||||
AuthValue := SecretStrSubstNo('Bearer %1', BearerToken);
|
||||
Client.DefaultRequestHeaders.Add('Authorization', AuthValue);
|
||||
Client.Get('https://api.example.com/data', Response);
|
||||
end;
|
||||
}
|
||||
12
samples/security/use-secrettext-with-httpclient/bad.al
Normal file
12
samples/security/use-secrettext-with-httpclient/bad.al
Normal file
|
|
@ -0,0 +1,12 @@
|
|||
codeunit 50213 "Sec Sample SecretHttpClient Bad"
|
||||
{
|
||||
procedure Call(ApiKey: Text)
|
||||
var
|
||||
Client: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
FullUrl: Text;
|
||||
begin
|
||||
FullUrl := 'https://api.example.com/v1?key=' + ApiKey;
|
||||
Client.Get(FullUrl, Response);
|
||||
end;
|
||||
}
|
||||
15
samples/security/use-secrettext-with-httpclient/good.al
Normal file
15
samples/security/use-secrettext-with-httpclient/good.al
Normal file
|
|
@ -0,0 +1,15 @@
|
|||
codeunit 50212 "Sec Sample SecretHttpClient Good"
|
||||
{
|
||||
procedure Call(ApiKey: SecretText)
|
||||
var
|
||||
Client: HttpClient;
|
||||
Request: HttpRequestMessage;
|
||||
Response: HttpResponseMessage;
|
||||
SecretUri: SecretText;
|
||||
begin
|
||||
SecretUri := SecretStrSubstNo('https://api.example.com/v1?key=%1', ApiKey);
|
||||
Request.SetSecretRequestUri(SecretUri);
|
||||
Request.Method('GET');
|
||||
Client.Send(Request, Response);
|
||||
end;
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue