From b6515ff12f5e697b919efcd915b61242c603a983 Mon Sep 17 00:00:00 2001 From: wenjiefan Date: Wed, 19 Aug 2026 14:52:31 +0200 Subject: [PATCH] Let a review comment declare several BCQuality articles A single expected comment can exercise more than one article, most often when it pairs a positive requirement with an explicit false-positive boundary (synthetic__error-handling-errorinfo-actionable-boundary-01, synthetic__events-ishandled-reset-boundary-01). The singular article field forced such findings to drop one of the two, and routing the overflow through metadata.articles is a dead end: the disjointness validator rejects it as soon as another comment in the same entry legitimately owns that article. Replace ReviewComment.article with articles: list[ArticleId] and migrate the 17 existing annotations. Set extra=forbid on the model so a leftover singular article key fails loudly instead of being silently ignored, which would otherwise turn a stale annotation into invisible coverage loss. Also refresh the notebook outputs, whose committed run still reported 130 entries. --- dataset/codereview.jsonl | 26 +++++++-------- notebooks/code-review-coverage.ipynb | 22 ++++++++++--- src/bcbench/dataset/codereview.py | 22 +++++++------ tests/test_bcquality_article_coverage.py | 40 ++++++++++++++++++------ 4 files changed, 74 insertions(+), 36 deletions(-) diff --git a/dataset/codereview.jsonl b/dataset/codereview.jsonl index fd5f5795f..97e6e3491 100644 --- a/dataset/codereview.jsonl +++ b/dataset/codereview.jsonl @@ -1,18 +1,18 @@ -{"repo": "microsoft/BCApps", "instance_id": "synthetic__security-001", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/SecureAPIManager.Codeunit.al b/src/SecureAPIManager.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/SecureAPIManager.Codeunit.al\n@@ -0,0 +1,46 @@\n+codeunit 50100 \"Secure API Manager\"\n+{\n+ Access = Internal;\n+\n+ var\n+ KeyNotConfiguredErr: Label 'API key is not configured for %1.', Comment = '%1 = configuration code';\n+ RequestFailedErr: Label 'The API request failed. Check the configuration.', Comment = 'Shown when an outbound API call fails.';\n+ EndpointTok: Label 'https://api.businesscentral.dynamics.com/v2.0/data', Locked = true;\n+ BearerTok: Label 'Bearer %1', Locked = true;\n+ StorageKeyTok: Label 'ApiKey_%1', Locked = true;\n+\n+ [NonDebuggable]\n+ procedure StoreKey(ConfigCode: Code[20]; KeyValue: SecretText)\n+ begin\n+ IsolatedStorage.SetEncrypted(GetStorageKey(ConfigCode), KeyValue, DataScope::Module);\n+ end;\n+\n+ [NonDebuggable]\n+ procedure GetKey(ConfigCode: Code[20]) Result: SecretText\n+ begin\n+ if not IsolatedStorage.Contains(GetStorageKey(ConfigCode), DataScope::Module) then\n+ Error(KeyNotConfiguredErr, ConfigCode);\n+ IsolatedStorage.Get(GetStorageKey(ConfigCode), DataScope::Module, Result);\n+ end;\n+\n+ procedure CallEndpoint(ConfigCode: Code[20])\n+ var\n+ Client: HttpClient;\n+ Headers: HttpHeaders;\n+ Response: HttpResponseMessage;\n+ AuthHeader: SecretText;\n+ begin\n+ AuthHeader := SecretStrSubstNo(BearerTok, GetKey(ConfigCode));\n+ Headers := Client.DefaultRequestHeaders();\n+ Headers.Add('Authorization', AuthHeader);\n+ if not Client.Get(EndpointTok, Response) then\n+ Error(RequestFailedErr);\n+ if not Response.IsSuccessStatusCode() then\n+ Error(RequestFailedErr);\n+ end;\n+\n+ local procedure GetStorageKey(ConfigCode: Code[20]): Text[50]\n+ begin\n+ exit(CopyStr(StrSubstNo(StorageKeyTok, ConfigCode), 1, 50));\n+ end;\n+}\ndiff --git a/src/HardcodedSecretClient.Codeunit.al b/src/HardcodedSecretClient.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/HardcodedSecretClient.Codeunit.al\n@@ -0,0 +1,18 @@\n+codeunit 50301 \"Hardcoded Secret Client\"\n+{\n+ procedure CallApi()\n+ var\n+ HttpClient: HttpClient;\n+ HttpHeaders: HttpHeaders;\n+ HttpResponseMessage: HttpResponseMessage;\n+ begin\n+ HttpHeaders := HttpClient.DefaultRequestHeaders();\n+ HttpHeaders.Add('X-Api-Key', this.GetApiKey());\n+ HttpClient.Get('https://api.contoso.com/data', HttpResponseMessage);\n+ end;\n+\n+ local procedure GetApiKey(): Text\n+ begin\n+ exit('sk-1234567890abcdef');\n+ end;\n+}\n", "expected_comments": [{"file": "src/HardcodedSecretClient.Codeunit.al", "line_start": 16, "line_end": 16, "severity": "critical", "domain": "security", "body": "Hardcoded API key in source code. Retrieve secrets from encrypted isolated storage or another secure store instead.", "article": "security/secrettext-for-credentials"}], "category": "code-review", "description": "Clean codeunit using SecretText, NonDebuggable, IsolatedStorage.SetEncrypted, and HTTPS enforcement with no security issues", "expect_findings": true, "source": "vsoadmin"} -{"repo": "microsoft/BCApps", "instance_id": "synthetic__security-002", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/SecureKeyManager.Codeunit.al b/src/SecureKeyManager.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/SecureKeyManager.Codeunit.al\n@@ -0,0 +1,26 @@\n+codeunit 50101 \"Secure Key Manager\"\n+{\n+ Access = Internal;\n+\n+ var\n+ KeyNotFoundErr: Label 'The requested key was not found. Configure it before use.';\n+\n+ [NonDebuggable]\n+ procedure StoreEncryptedKey(KeyName: Code[20]; KeyValue: SecretText)\n+ begin\n+ IsolatedStorage.SetEncrypted(KeyName, KeyValue, DataScope::Module);\n+ end;\n+\n+ [NonDebuggable]\n+ procedure RetrieveKey(KeyName: Code[20]) Result: SecretText\n+ begin\n+ if not IsolatedStorage.Contains(KeyName, DataScope::Module) then\n+ Error(KeyNotFoundErr);\n+ IsolatedStorage.Get(KeyName, DataScope::Module, Result);\n+ end;\n+\n+ procedure HasKey(KeyName: Code[20]): Boolean\n+ begin\n+ exit(IsolatedStorage.Contains(KeyName, DataScope::Module));\n+ end;\n+}\ndiff --git a/src/PlainTokenHeaderClient.Codeunit.al b/src/PlainTokenHeaderClient.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/PlainTokenHeaderClient.Codeunit.al\n@@ -0,0 +1,18 @@\n+codeunit 50302 \"Plain Token Header Client\"\n+{\n+ procedure SendRequest(AccessToken: Text)\n+ var\n+ HttpClient: HttpClient;\n+ HttpHeaders: HttpHeaders;\n+ HttpResponseMessage: HttpResponseMessage;\n+ begin\n+ HttpHeaders := HttpClient.DefaultRequestHeaders();\n+ HttpHeaders.Add('Authorization', 'Bearer ' + AccessToken);\n+ HttpClient.Get(this.GetOrdersEndpoint(), HttpResponseMessage);\n+ end;\n+\n+ local procedure GetOrdersEndpoint(): Text\n+ begin\n+ exit('https://api.contoso.com/orders');\n+ end;\n+}\n", "expected_comments": [{"file": "src/PlainTokenHeaderClient.Codeunit.al", "line_start": 10, "line_end": 10, "severity": "high", "domain": "security", "body": "Bearer token is concatenated into a plain Text authorization header. Build the header with SecretStrSubstNo() and add it as SecretText.", "article": "security/secretstrsubstno-for-composing-secrets"}], "category": "code-review", "description": "Clean codeunit correctly storing and retrieving API keys using IsolatedStorage.SetEncrypted, SecretText, and NonDebuggable", "expect_findings": true, "source": "vsoadmin"} -{"repo": "microsoft/BCApps", "instance_id": "synthetic__security-003", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/SafeErrorHandler.Codeunit.al b/src/SafeErrorHandler.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/SafeErrorHandler.Codeunit.al\n@@ -0,0 +1,41 @@\n+codeunit 50102 \"Safe Error Handler\"\n+{\n+ Access = Internal;\n+\n+ var\n+ InvalidRequestTxt: Label 'Invalid request. Please check your input.';\n+ AuthFailedTxt: Label 'Authentication failed. Please verify your credentials.';\n+ ForbiddenTxt: Label 'You do not have permission for this operation.';\n+ NotFoundTxt: Label 'The requested resource was not found.';\n+ UnexpectedTxt: Label 'An unexpected error occurred. Contact your administrator.';\n+ PostFailedErr: Label 'Could not post document %1.', Comment = '%1 = document number';\n+\n+ procedure GetApiResponseMessage(StatusCode: Integer): Text\n+ begin\n+ case StatusCode of\n+ 200, 201:\n+ exit('');\n+ 400:\n+ exit(InvalidRequestTxt);\n+ 401:\n+ exit(AuthFailedTxt);\n+ 403:\n+ exit(ForbiddenTxt);\n+ 404:\n+ exit(NotFoundTxt);\n+ else\n+ exit(UnexpectedTxt);\n+ end;\n+ end;\n+\n+ procedure PostDocument(DocNo: Code[20])\n+ begin\n+ if not TryPost(DocNo) then\n+ Error(PostFailedErr, DocNo);\n+ end;\n+\n+ [TryFunction]\n+ local procedure TryPost(DocNo: Code[20])\n+ begin\n+ end;\n+}\ndiff --git a/src/UnwrappedSecretClient.Codeunit.al b/src/UnwrappedSecretClient.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/UnwrappedSecretClient.Codeunit.al\n@@ -0,0 +1,18 @@\n+codeunit 50303 \"Unwrapped Secret Client\"\n+{\n+ procedure SendRequest(SessionToken: SecretText)\n+ var\n+ HttpClient: HttpClient;\n+ HttpHeaders: HttpHeaders;\n+ HttpResponseMessage: HttpResponseMessage;\n+ begin\n+ HttpHeaders := HttpClient.DefaultRequestHeaders();\n+ HttpHeaders.Add('Authorization', this.BuildHeader(SessionToken));\n+ HttpClient.Get('https://api.contoso.com/data', HttpResponseMessage);\n+ end;\n+\n+ local procedure BuildHeader(SessionToken: SecretText): Text\n+ begin\n+ exit('Bearer ' + SessionToken.Unwrap());\n+ end;\n+}\n", "expected_comments": [{"file": "src/UnwrappedSecretClient.Codeunit.al", "line_start": 16, "line_end": 16, "severity": "high", "domain": "security", "body": "SecretText.Unwrap() exposes the secret as plain Text without a [NonDebuggable] procedure. Add [NonDebuggable] or avoid unwrapping by using SecretStrSubstNo().", "article": "security/nondebuggable-required-when-unwrapping-secrettext"}], "category": "code-review", "description": "Clean codeunit with proper error handling: generic user-facing messages, no system details exposed, no GetLastErrorText shown to user", "expect_findings": true, "source": "vsoadmin"} -{"repo": "microsoft/BCApps", "instance_id": "synthetic__security-004", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/AppConstants.Codeunit.al b/src/AppConstants.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/AppConstants.Codeunit.al\n@@ -0,0 +1,30 @@\n+codeunit 50103 \"App Constants\"\n+{\n+ Access = Internal;\n+\n+ var\n+ ApiVersionTok: Label 'v2.0', Locked = true;\n+ DefaultCurrencyTok: Label 'USD', Locked = true;\n+ DateFormatTok: Label 'yyyy-MM-dd', Locked = true;\n+ AppIdTok: Label 'BC-INVENTORY-APP', Locked = true;\n+\n+ procedure GetApiVersion(): Text\n+ begin\n+ exit(ApiVersionTok);\n+ end;\n+\n+ procedure GetDefaultCurrency(): Code[10]\n+ begin\n+ exit(DefaultCurrencyTok);\n+ end;\n+\n+ procedure GetDateFormat(): Text\n+ begin\n+ exit(DateFormatTok);\n+ end;\n+\n+ procedure GetAppId(): Text\n+ begin\n+ exit(AppIdTok);\n+ end;\n+}\ndiff --git a/src/QueryStringSecretClient.Codeunit.al b/src/QueryStringSecretClient.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/QueryStringSecretClient.Codeunit.al\n@@ -0,0 +1,15 @@\n+codeunit 50304 \"Query String Secret Client\"\n+{\n+ procedure FetchAccount(ApiKey: Text)\n+ var\n+ HttpClient: HttpClient;\n+ HttpResponseMessage: HttpResponseMessage;\n+ begin\n+ HttpClient.Get(this.BuildAccountUrl(ApiKey), HttpResponseMessage);\n+ end;\n+\n+ local procedure BuildAccountUrl(ApiKey: Text): Text\n+ begin\n+ exit('https://api.contoso.com/accounts?api_key=' + ApiKey);\n+ end;\n+}\n", "expected_comments": [{"file": "src/QueryStringSecretClient.Codeunit.al", "line_start": 13, "line_end": 13, "severity": "high", "domain": "security", "body": "API key is placed in the URL query string. Use an Authorization header, or SetSecretRequestUri() if a secret URI is unavoidable.", "article": "security/secrettext-with-httpclient"}], "category": "code-review", "description": "Clean codeunit with configuration constants that are not secrets: API version, currency codes, labels, and format strings", "expect_findings": true, "source": "vsoadmin"} -{"repo": "microsoft/BCApps", "instance_id": "synthetic__security-005", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/ValidatedImportConfig.Table.al b/src/ValidatedImportConfig.Table.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/ValidatedImportConfig.Table.al\n@@ -0,0 +1,34 @@\n+table 50104 \"Validated Import Config\"\n+{\n+ Caption = 'Validated Import Configuration';\n+ DataClassification = CustomerContent;\n+\n+ fields\n+ {\n+ field(1; \"Code\"; Code[20])\n+ {\n+ Caption = 'Code';\n+ NotBlank = true;\n+ }\n+ field(2; \"Source Table ID\"; Integer)\n+ {\n+ Caption = 'Source Table';\n+ TableRelation = AllObjWithCaption.\"Object ID\" where(\"Object Type\" = const(Table));\n+ ValidateTableRelation = true;\n+ }\n+ field(3; \"Max Records\"; Integer)\n+ {\n+ Caption = 'Maximum Records';\n+ MinValue = 1;\n+ MaxValue = 10000;\n+ }\n+ }\n+\n+ keys\n+ {\n+ key(PK; \"Code\")\n+ {\n+ Clustered = true;\n+ }\n+ }\n+}\ndiff --git a/src/BroadFinanceAccess.PermissionSet.al b/src/BroadFinanceAccess.PermissionSet.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/BroadFinanceAccess.PermissionSet.al\n@@ -0,0 +1,15 @@\n+permissionset 50305 \"Broad Finance Access\"\n+{\n+ Assignable = true;\n+ Caption = 'Broad Finance Access', Locked = true;\n+ Permissions =\n+ tabledata * = RIMD,\n+ table * = X,\n+ tabledata Customer = R,\n+ tabledata Vendor = R,\n+ tabledata Item = R,\n+ tabledata \"Sales Header\" = R,\n+ tabledata \"Sales Line\" = R,\n+ codeunit \"Release Sales Document\" = X,\n+ codeunit \"Sales-Post\" = X;\n+}\n", "expected_comments": [{"file": "src/BroadFinanceAccess.PermissionSet.al", "line_start": 6, "line_end": 6, "severity": "critical", "domain": "security", "body": "Permission set grants RIMD on all table data. Replace the wildcard with the minimum specific tabledata permissions required.", "article": "security/permission-set-avoid-wildcard-grants"}, {"file": "src/BroadFinanceAccess.PermissionSet.al", "line_start": 7, "line_end": 7, "severity": "high", "domain": "security", "body": "Permission set grants execute permission on all tables. Grant execute only on the specific objects this role requires.", "article": "security/permission-set-avoid-wildcard-grants"}], "category": "code-review", "description": "Clean table with proper input validation: ValidateTableRelation, OnValidate triggers, MinValue/MaxValue, Editable=false on system fields", "expect_findings": true, "source": "vsoadmin"} -{"repo": "microsoft/BCApps", "instance_id": "synthetic__security-006", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/InventoryReader.PermissionSet.al b/src/InventoryReader.PermissionSet.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/InventoryReader.PermissionSet.al\n@@ -0,0 +1,11 @@\n+permissionset 50106 \"Inventory Reader\"\n+{\n+ Caption = 'Inventory Reader';\n+ Assignable = true;\n+\n+ Permissions =\n+ tabledata Item = r,\n+ tabledata \"Item Ledger Entry\" = r,\n+ tabledata \"Item Category\" = r,\n+ codeunit \"Inventory Lookup\" = X;\n+}\ndiff --git a/src/InventoryLookup.Codeunit.al b/src/InventoryLookup.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/InventoryLookup.Codeunit.al\n@@ -0,0 +1,15 @@\n+codeunit 50108 \"Inventory Lookup\"\n+{\n+ Access = Internal;\n+ Permissions = tabledata Item = r;\n+\n+ procedure GetItemDescription(ItemNo: Code[20]): Text[100]\n+ var\n+ Item: Record Item;\n+ begin\n+ Item.SetLoadFields(Description);\n+ if Item.Get(ItemNo) then\n+ exit(Item.Description);\n+ exit('');\n+ end;\n+}\ndiff --git a/src/ExcessiveInherentAccess.Codeunit.al b/src/ExcessiveInherentAccess.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/ExcessiveInherentAccess.Codeunit.al\n@@ -0,0 +1,17 @@\n+codeunit 50306 \"Excessive Inherent Access\"\n+{\n+ procedure LookupCustomerName(CustomerNo: Code[20]): Text\n+ begin\n+ exit(this.GetCustomerName(CustomerNo));\n+ end;\n+\n+ [InherentPermissions(PermissionObjectType::TableData, Database::Customer, 'RIMD')]\n+ local procedure GetCustomerName(CustomerNo: Code[20]): Text\n+ var\n+ Customer: Record Customer;\n+ begin\n+ if Customer.Get(CustomerNo) then\n+ exit(Customer.Name);\n+ exit('');\n+ end;\n+}\n", "expected_comments": [{"file": "src/ExcessiveInherentAccess.Codeunit.al", "line_start": 8, "line_end": 8, "severity": "high", "domain": "security", "body": "InherentPermissions grants RIMD tabledata access even though this procedure only reads Customer. Reduce the permission to the minimal read access required.", "article": "security/inherent-permissions-minimal-grant"}], "category": "code-review", "description": "Clean permission sets with least-privilege access: read-only for readers, read-insert for editors, no RIMD grants", "expect_findings": true, "source": "vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__security-001","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-05-15T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/SecureAPIManager.Codeunit.al b/src/SecureAPIManager.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/SecureAPIManager.Codeunit.al\n@@ -0,0 +1,46 @@\n+codeunit 50100 \"Secure API Manager\"\n+{\n+ Access = Internal;\n+\n+ var\n+ KeyNotConfiguredErr: Label 'API key is not configured for %1.', Comment = '%1 = configuration code';\n+ RequestFailedErr: Label 'The API request failed. Check the configuration.', Comment = 'Shown when an outbound API call fails.';\n+ EndpointTok: Label 'https://api.businesscentral.dynamics.com/v2.0/data', Locked = true;\n+ BearerTok: Label 'Bearer %1', Locked = true;\n+ StorageKeyTok: Label 'ApiKey_%1', Locked = true;\n+\n+ [NonDebuggable]\n+ procedure StoreKey(ConfigCode: Code[20]; KeyValue: SecretText)\n+ begin\n+ IsolatedStorage.SetEncrypted(GetStorageKey(ConfigCode), KeyValue, DataScope::Module);\n+ end;\n+\n+ [NonDebuggable]\n+ procedure GetKey(ConfigCode: Code[20]) Result: SecretText\n+ begin\n+ if not IsolatedStorage.Contains(GetStorageKey(ConfigCode), DataScope::Module) then\n+ Error(KeyNotConfiguredErr, ConfigCode);\n+ IsolatedStorage.Get(GetStorageKey(ConfigCode), DataScope::Module, Result);\n+ end;\n+\n+ procedure CallEndpoint(ConfigCode: Code[20])\n+ var\n+ Client: HttpClient;\n+ Headers: HttpHeaders;\n+ Response: HttpResponseMessage;\n+ AuthHeader: SecretText;\n+ begin\n+ AuthHeader := SecretStrSubstNo(BearerTok, GetKey(ConfigCode));\n+ Headers := Client.DefaultRequestHeaders();\n+ Headers.Add('Authorization', AuthHeader);\n+ if not Client.Get(EndpointTok, Response) then\n+ Error(RequestFailedErr);\n+ if not Response.IsSuccessStatusCode() then\n+ Error(RequestFailedErr);\n+ end;\n+\n+ local procedure GetStorageKey(ConfigCode: Code[20]): Text[50]\n+ begin\n+ exit(CopyStr(StrSubstNo(StorageKeyTok, ConfigCode), 1, 50));\n+ end;\n+}\ndiff --git a/src/HardcodedSecretClient.Codeunit.al b/src/HardcodedSecretClient.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/HardcodedSecretClient.Codeunit.al\n@@ -0,0 +1,18 @@\n+codeunit 50301 \"Hardcoded Secret Client\"\n+{\n+ procedure CallApi()\n+ var\n+ HttpClient: HttpClient;\n+ HttpHeaders: HttpHeaders;\n+ HttpResponseMessage: HttpResponseMessage;\n+ begin\n+ HttpHeaders := HttpClient.DefaultRequestHeaders();\n+ HttpHeaders.Add('X-Api-Key', this.GetApiKey());\n+ HttpClient.Get('https://api.contoso.com/data', HttpResponseMessage);\n+ end;\n+\n+ local procedure GetApiKey(): Text\n+ begin\n+ exit('sk-1234567890abcdef');\n+ end;\n+}\n","expected_comments":[{"file":"src/HardcodedSecretClient.Codeunit.al","line_start":16,"line_end":16,"severity":"critical","domain":"security","body":"Hardcoded API key in source code. Retrieve secrets from encrypted isolated storage or another secure store instead.","articles":["security/secrettext-for-credentials"]}],"category":"code-review","description":"Clean codeunit using SecretText, NonDebuggable, IsolatedStorage.SetEncrypted, and HTTPS enforcement with no security issues","expect_findings":true,"source":"vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__security-002","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-05-15T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/SecureKeyManager.Codeunit.al b/src/SecureKeyManager.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/SecureKeyManager.Codeunit.al\n@@ -0,0 +1,26 @@\n+codeunit 50101 \"Secure Key Manager\"\n+{\n+ Access = Internal;\n+\n+ var\n+ KeyNotFoundErr: Label 'The requested key was not found. Configure it before use.';\n+\n+ [NonDebuggable]\n+ procedure StoreEncryptedKey(KeyName: Code[20]; KeyValue: SecretText)\n+ begin\n+ IsolatedStorage.SetEncrypted(KeyName, KeyValue, DataScope::Module);\n+ end;\n+\n+ [NonDebuggable]\n+ procedure RetrieveKey(KeyName: Code[20]) Result: SecretText\n+ begin\n+ if not IsolatedStorage.Contains(KeyName, DataScope::Module) then\n+ Error(KeyNotFoundErr);\n+ IsolatedStorage.Get(KeyName, DataScope::Module, Result);\n+ end;\n+\n+ procedure HasKey(KeyName: Code[20]): Boolean\n+ begin\n+ exit(IsolatedStorage.Contains(KeyName, DataScope::Module));\n+ end;\n+}\ndiff --git a/src/PlainTokenHeaderClient.Codeunit.al b/src/PlainTokenHeaderClient.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/PlainTokenHeaderClient.Codeunit.al\n@@ -0,0 +1,18 @@\n+codeunit 50302 \"Plain Token Header Client\"\n+{\n+ procedure SendRequest(AccessToken: Text)\n+ var\n+ HttpClient: HttpClient;\n+ HttpHeaders: HttpHeaders;\n+ HttpResponseMessage: HttpResponseMessage;\n+ begin\n+ HttpHeaders := HttpClient.DefaultRequestHeaders();\n+ HttpHeaders.Add('Authorization', 'Bearer ' + AccessToken);\n+ HttpClient.Get(this.GetOrdersEndpoint(), HttpResponseMessage);\n+ end;\n+\n+ local procedure GetOrdersEndpoint(): Text\n+ begin\n+ exit('https://api.contoso.com/orders');\n+ end;\n+}\n","expected_comments":[{"file":"src/PlainTokenHeaderClient.Codeunit.al","line_start":10,"line_end":10,"severity":"high","domain":"security","body":"Bearer token is concatenated into a plain Text authorization header. Build the header with SecretStrSubstNo() and add it as SecretText.","articles":["security/secretstrsubstno-for-composing-secrets"]}],"category":"code-review","description":"Clean codeunit correctly storing and retrieving API keys using IsolatedStorage.SetEncrypted, SecretText, and NonDebuggable","expect_findings":true,"source":"vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__security-003","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-05-15T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/SafeErrorHandler.Codeunit.al b/src/SafeErrorHandler.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/SafeErrorHandler.Codeunit.al\n@@ -0,0 +1,41 @@\n+codeunit 50102 \"Safe Error Handler\"\n+{\n+ Access = Internal;\n+\n+ var\n+ InvalidRequestTxt: Label 'Invalid request. Please check your input.';\n+ AuthFailedTxt: Label 'Authentication failed. Please verify your credentials.';\n+ ForbiddenTxt: Label 'You do not have permission for this operation.';\n+ NotFoundTxt: Label 'The requested resource was not found.';\n+ UnexpectedTxt: Label 'An unexpected error occurred. Contact your administrator.';\n+ PostFailedErr: Label 'Could not post document %1.', Comment = '%1 = document number';\n+\n+ procedure GetApiResponseMessage(StatusCode: Integer): Text\n+ begin\n+ case StatusCode of\n+ 200, 201:\n+ exit('');\n+ 400:\n+ exit(InvalidRequestTxt);\n+ 401:\n+ exit(AuthFailedTxt);\n+ 403:\n+ exit(ForbiddenTxt);\n+ 404:\n+ exit(NotFoundTxt);\n+ else\n+ exit(UnexpectedTxt);\n+ end;\n+ end;\n+\n+ procedure PostDocument(DocNo: Code[20])\n+ begin\n+ if not TryPost(DocNo) then\n+ Error(PostFailedErr, DocNo);\n+ end;\n+\n+ [TryFunction]\n+ local procedure TryPost(DocNo: Code[20])\n+ begin\n+ end;\n+}\ndiff --git a/src/UnwrappedSecretClient.Codeunit.al b/src/UnwrappedSecretClient.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/UnwrappedSecretClient.Codeunit.al\n@@ -0,0 +1,18 @@\n+codeunit 50303 \"Unwrapped Secret Client\"\n+{\n+ procedure SendRequest(SessionToken: SecretText)\n+ var\n+ HttpClient: HttpClient;\n+ HttpHeaders: HttpHeaders;\n+ HttpResponseMessage: HttpResponseMessage;\n+ begin\n+ HttpHeaders := HttpClient.DefaultRequestHeaders();\n+ HttpHeaders.Add('Authorization', this.BuildHeader(SessionToken));\n+ HttpClient.Get('https://api.contoso.com/data', HttpResponseMessage);\n+ end;\n+\n+ local procedure BuildHeader(SessionToken: SecretText): Text\n+ begin\n+ exit('Bearer ' + SessionToken.Unwrap());\n+ end;\n+}\n","expected_comments":[{"file":"src/UnwrappedSecretClient.Codeunit.al","line_start":16,"line_end":16,"severity":"high","domain":"security","body":"SecretText.Unwrap() exposes the secret as plain Text without a [NonDebuggable] procedure. Add [NonDebuggable] or avoid unwrapping by using SecretStrSubstNo().","articles":["security/nondebuggable-required-when-unwrapping-secrettext"]}],"category":"code-review","description":"Clean codeunit with proper error handling: generic user-facing messages, no system details exposed, no GetLastErrorText shown to user","expect_findings":true,"source":"vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__security-004","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-05-15T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/AppConstants.Codeunit.al b/src/AppConstants.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/AppConstants.Codeunit.al\n@@ -0,0 +1,30 @@\n+codeunit 50103 \"App Constants\"\n+{\n+ Access = Internal;\n+\n+ var\n+ ApiVersionTok: Label 'v2.0', Locked = true;\n+ DefaultCurrencyTok: Label 'USD', Locked = true;\n+ DateFormatTok: Label 'yyyy-MM-dd', Locked = true;\n+ AppIdTok: Label 'BC-INVENTORY-APP', Locked = true;\n+\n+ procedure GetApiVersion(): Text\n+ begin\n+ exit(ApiVersionTok);\n+ end;\n+\n+ procedure GetDefaultCurrency(): Code[10]\n+ begin\n+ exit(DefaultCurrencyTok);\n+ end;\n+\n+ procedure GetDateFormat(): Text\n+ begin\n+ exit(DateFormatTok);\n+ end;\n+\n+ procedure GetAppId(): Text\n+ begin\n+ exit(AppIdTok);\n+ end;\n+}\ndiff --git a/src/QueryStringSecretClient.Codeunit.al b/src/QueryStringSecretClient.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/QueryStringSecretClient.Codeunit.al\n@@ -0,0 +1,15 @@\n+codeunit 50304 \"Query String Secret Client\"\n+{\n+ procedure FetchAccount(ApiKey: Text)\n+ var\n+ HttpClient: HttpClient;\n+ HttpResponseMessage: HttpResponseMessage;\n+ begin\n+ HttpClient.Get(this.BuildAccountUrl(ApiKey), HttpResponseMessage);\n+ end;\n+\n+ local procedure BuildAccountUrl(ApiKey: Text): Text\n+ begin\n+ exit('https://api.contoso.com/accounts?api_key=' + ApiKey);\n+ end;\n+}\n","expected_comments":[{"file":"src/QueryStringSecretClient.Codeunit.al","line_start":13,"line_end":13,"severity":"high","domain":"security","body":"API key is placed in the URL query string. Use an Authorization header, or SetSecretRequestUri() if a secret URI is unavoidable.","articles":["security/secrettext-with-httpclient"]}],"category":"code-review","description":"Clean codeunit with configuration constants that are not secrets: API version, currency codes, labels, and format strings","expect_findings":true,"source":"vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__security-005","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-05-15T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/ValidatedImportConfig.Table.al b/src/ValidatedImportConfig.Table.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/ValidatedImportConfig.Table.al\n@@ -0,0 +1,34 @@\n+table 50104 \"Validated Import Config\"\n+{\n+ Caption = 'Validated Import Configuration';\n+ DataClassification = CustomerContent;\n+\n+ fields\n+ {\n+ field(1; \"Code\"; Code[20])\n+ {\n+ Caption = 'Code';\n+ NotBlank = true;\n+ }\n+ field(2; \"Source Table ID\"; Integer)\n+ {\n+ Caption = 'Source Table';\n+ TableRelation = AllObjWithCaption.\"Object ID\" where(\"Object Type\" = const(Table));\n+ ValidateTableRelation = true;\n+ }\n+ field(3; \"Max Records\"; Integer)\n+ {\n+ Caption = 'Maximum Records';\n+ MinValue = 1;\n+ MaxValue = 10000;\n+ }\n+ }\n+\n+ keys\n+ {\n+ key(PK; \"Code\")\n+ {\n+ Clustered = true;\n+ }\n+ }\n+}\ndiff --git a/src/BroadFinanceAccess.PermissionSet.al b/src/BroadFinanceAccess.PermissionSet.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/BroadFinanceAccess.PermissionSet.al\n@@ -0,0 +1,15 @@\n+permissionset 50305 \"Broad Finance Access\"\n+{\n+ Assignable = true;\n+ Caption = 'Broad Finance Access', Locked = true;\n+ Permissions =\n+ tabledata * = RIMD,\n+ table * = X,\n+ tabledata Customer = R,\n+ tabledata Vendor = R,\n+ tabledata Item = R,\n+ tabledata \"Sales Header\" = R,\n+ tabledata \"Sales Line\" = R,\n+ codeunit \"Release Sales Document\" = X,\n+ codeunit \"Sales-Post\" = X;\n+}\n","expected_comments":[{"file":"src/BroadFinanceAccess.PermissionSet.al","line_start":6,"line_end":6,"severity":"critical","domain":"security","body":"Permission set grants RIMD on all table data. Replace the wildcard with the minimum specific tabledata permissions required.","articles":["security/permission-set-avoid-wildcard-grants"]},{"file":"src/BroadFinanceAccess.PermissionSet.al","line_start":7,"line_end":7,"severity":"high","domain":"security","body":"Permission set grants execute permission on all tables. Grant execute only on the specific objects this role requires.","articles":["security/permission-set-avoid-wildcard-grants"]}],"category":"code-review","description":"Clean table with proper input validation: ValidateTableRelation, OnValidate triggers, MinValue/MaxValue, Editable=false on system fields","expect_findings":true,"source":"vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__security-006","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-05-15T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/InventoryReader.PermissionSet.al b/src/InventoryReader.PermissionSet.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/InventoryReader.PermissionSet.al\n@@ -0,0 +1,11 @@\n+permissionset 50106 \"Inventory Reader\"\n+{\n+ Caption = 'Inventory Reader';\n+ Assignable = true;\n+\n+ Permissions =\n+ tabledata Item = r,\n+ tabledata \"Item Ledger Entry\" = r,\n+ tabledata \"Item Category\" = r,\n+ codeunit \"Inventory Lookup\" = X;\n+}\ndiff --git a/src/InventoryLookup.Codeunit.al b/src/InventoryLookup.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/InventoryLookup.Codeunit.al\n@@ -0,0 +1,15 @@\n+codeunit 50108 \"Inventory Lookup\"\n+{\n+ Access = Internal;\n+ Permissions = tabledata Item = r;\n+\n+ procedure GetItemDescription(ItemNo: Code[20]): Text[100]\n+ var\n+ Item: Record Item;\n+ begin\n+ Item.SetLoadFields(Description);\n+ if Item.Get(ItemNo) then\n+ exit(Item.Description);\n+ exit('');\n+ end;\n+}\ndiff --git a/src/ExcessiveInherentAccess.Codeunit.al b/src/ExcessiveInherentAccess.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/ExcessiveInherentAccess.Codeunit.al\n@@ -0,0 +1,17 @@\n+codeunit 50306 \"Excessive Inherent Access\"\n+{\n+ procedure LookupCustomerName(CustomerNo: Code[20]): Text\n+ begin\n+ exit(this.GetCustomerName(CustomerNo));\n+ end;\n+\n+ [InherentPermissions(PermissionObjectType::TableData, Database::Customer, 'RIMD')]\n+ local procedure GetCustomerName(CustomerNo: Code[20]): Text\n+ var\n+ Customer: Record Customer;\n+ begin\n+ if Customer.Get(CustomerNo) then\n+ exit(Customer.Name);\n+ exit('');\n+ end;\n+}\n","expected_comments":[{"file":"src/ExcessiveInherentAccess.Codeunit.al","line_start":8,"line_end":8,"severity":"high","domain":"security","body":"InherentPermissions grants RIMD tabledata access even though this procedure only reads Customer. Reduce the permission to the minimal read access required.","articles":["security/inherent-permissions-minimal-grant"]}],"category":"code-review","description":"Clean permission sets with least-privilege access: read-only for readers, read-insert for editors, no RIMD grants","expect_findings":true,"source":"vsoadmin"} {"repo": "microsoft/BCApps", "instance_id": "synthetic__security-007", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/SafeRecordQuery.Codeunit.al b/src/SafeRecordQuery.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/SafeRecordQuery.Codeunit.al\n@@ -0,0 +1,31 @@\n+codeunit 50109 \"Safe Record Query\"\n+{\n+ Access = Internal;\n+\n+ procedure CustomerExists(CustomerNo: Code[20]): Boolean\n+ var\n+ Customer: Record Customer;\n+ begin\n+ Customer.SetLoadFields(\"No.\");\n+ exit(Customer.Get(CustomerNo));\n+ end;\n+\n+ procedure HasOpenSalesOrders(CustomerNo: Code[20]): Boolean\n+ var\n+ SalesHeader: Record \"Sales Header\";\n+ begin\n+ SalesHeader.SetRange(\"Document Type\", SalesHeader.\"Document Type\"::Order);\n+ SalesHeader.SetRange(\"Sell-to Customer No.\", CustomerNo);\n+ exit(not SalesHeader.IsEmpty());\n+ end;\n+\n+ procedure GetItemDescription(ItemNo: Code[20]): Text[100]\n+ var\n+ Item: Record Item;\n+ begin\n+ Item.SetLoadFields(Description);\n+ if Item.Get(ItemNo) then\n+ exit(Item.Description);\n+ exit('');\n+ end;\n+}\ndiff --git a/src/InsecureEndpointClient.Codeunit.al b/src/InsecureEndpointClient.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/InsecureEndpointClient.Codeunit.al\n@@ -0,0 +1,17 @@\n+codeunit 50307 \"Insecure Endpoint Client\"\n+{\n+ procedure SendSession(SessionToken: SecretText)\n+ var\n+ HttpClient: HttpClient;\n+ HttpContent: HttpContent;\n+ HttpResponseMessage: HttpResponseMessage;\n+ begin\n+ HttpContent.WriteFrom(SessionToken);\n+ HttpClient.Post(this.GetEndpoint(), HttpContent, HttpResponseMessage);\n+ end;\n+\n+ local procedure GetEndpoint(): Text\n+ begin\n+ exit('http://api.contoso.com/session');\n+ end;\n+}\n", "expected_comments": [{"file": "src/InsecureEndpointClient.Codeunit.al", "line_start": 15, "line_end": 15, "severity": "high", "domain": "security", "body": "External service endpoint uses HTTP instead of HTTPS. Use HTTPS for all external HTTP calls, especially when sending session tokens."}], "category": "code-review", "description": "Clean codeunit using proper BC record operations: SetRange, SetFilter, FindSet, Count — no string concatenation or dynamic SQL", "expect_findings": true, "source": "vsoadmin"} -{"repo": "microsoft/BCApps", "instance_id": "synthetic__security-008", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/PartnerConfigKeyManager.Codeunit.al b/src/PartnerConfigKeyManager.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/PartnerConfigKeyManager.Codeunit.al\n@@ -0,0 +1,9 @@\n+codeunit 50100 \"Partner Config Key Manager\"\n+{\n+ Access = Internal;\n+\n+ internal procedure StoreApiKey(KeyName: Text; ApiKey: Text)\n+ begin\n+ IsolatedStorage.SetEncrypted(KeyName, ApiKey, DataScope::Module);\n+ end;\n+}\n", "expected_comments": [{"file": "src/PartnerConfigKeyManager.Codeunit.al", "line_start": 5, "line_end": 5, "domain": "security", "severity": "high", "body": "The API key is accepted as a plain Text parameter instead of SecretText, so the secret is exposed in memory and to anyone inspecting the call stack or debugger.", "article": "security/secrettext-for-credentials"}], "category": "code-review", "description": "True positive security findings: encryption (trimmed to 5 representative findings)", "expect_findings": true, "source": "vsoadmin"} -{"repo": "microsoft/BCApps", "instance_id": "synthetic__security-009", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/OutlookAddinDeployer.Codeunit.al b/src/OutlookAddinDeployer.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/OutlookAddinDeployer.Codeunit.al\n@@ -0,0 +1,37 @@\n+namespace Microsoft.Integration.Outlook;\n+\n+codeunit 50104 \"Outlook Addin Deployer\"\n+{\n+ Access = Internal;\n+\n+ var\n+ EndpointTok: Label 'https://outlook.office365.com/api/v2.0/addins/deploy', Locked = true;\n+ StatusErr: Label 'Deployment failed (HTTP %1): %2', Comment = '%1 is the HTTP status code, %2 is the raw response body.';\n+ ConnectErr: Label 'Failed to connect to the deployment service: %1', Comment = '%1 is the underlying error text.';\n+\n+ procedure DeployAddin(ManifestPath: Text)\n+ var\n+ Client: HttpClient;\n+ Content: HttpContent;\n+ Response: HttpResponseMessage;\n+ Headers: HttpHeaders;\n+ Payload: Text;\n+ ResponseText: Text;\n+ begin\n+ Payload := '{\"manifest\":\"' + ManifestPath + '\"}';\n+ Content.WriteFrom(Payload);\n+ Content.GetHeaders(Headers);\n+ Headers.Add('Authorization', 'Bearer ' + GetAccessToken());\n+ if Client.Post(EndpointTok, Content, Response) then begin\n+ Response.Content.ReadAs(ResponseText);\n+ if not Response.IsSuccessStatusCode() then\n+ Error(StatusErr, Response.HttpStatusCode(), ResponseText);\n+ end else\n+ Error(ConnectErr, GetLastErrorText());\n+ end;\n+\n+ local procedure GetAccessToken(): Text\n+ begin\n+ exit('dummy_access_token_for_testing');\n+ end;\n+}\n", "expected_comments": [{"file": "src/OutlookAddinDeployer.Codeunit.al", "line_start": 21, "line_end": 21, "domain": "security", "severity": "medium", "body": "The manifest path is concatenated directly into a JSON payload, allowing JSON injection. Build the payload with a JsonObject so values are escaped."}, {"file": "src/OutlookAddinDeployer.Codeunit.al", "line_start": 28, "line_end": 28, "domain": "security", "severity": "medium", "body": "The error surfaces the raw HTTP status code and full response body to the user, leaking internal service details. Log the details and show a generic message."}, {"file": "src/OutlookAddinDeployer.Codeunit.al", "line_start": 30, "line_end": 30, "domain": "security", "severity": "medium", "body": "GetLastErrorText() is shown to the user, exposing internal system details. Log the raw error and present a sanitized message."}, {"file": "src/OutlookAddinDeployer.Codeunit.al", "line_start": 33, "line_end": 33, "domain": "security", "severity": "high", "body": "The access token is returned as plain Text instead of SecretText, exposing it in memory and to the debugger. Return and handle it as SecretText.", "article": "security/secrettext-for-credentials"}, {"file": "src/OutlookAddinDeployer.Codeunit.al", "line_start": 35, "line_end": 35, "domain": "security", "severity": "high", "body": "A hardcoded access token is embedded in source code. Retrieve the token from a secure store or OAuth flow instead of hardcoding it.", "article": "security/secrettext-for-credentials"}], "category": "code-review", "description": "True positive security findings: error_exposure (verified line numbers)", "expect_findings": true, "source": "vsoadmin"} -{"repo": "microsoft/BCApps", "instance_id": "synthetic__security-011", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/ElecVATSubmission.Codeunit.al b/src/ElecVATSubmission.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/ElecVATSubmission.Codeunit.al\n@@ -0,0 +1,24 @@\n+namespace Microsoft.Finance.VAT;\n+\n+codeunit 13610 \"Elec VAT Submission\"\n+{\n+ Access = Internal;\n+\n+ procedure SubmitReturn(AuthorityUrl: Text): Boolean\n+ var\n+ Client: HttpClient;\n+ Content: HttpContent;\n+ Response: HttpResponseMessage;\n+ begin\n+ Content.WriteFrom('{}');\n+ exit(Client.Post(AuthorityUrl, Content, Response));\n+ end;\n+\n+ procedure CheckHealth(ServiceUrl: Text): Boolean\n+ var\n+ Client: HttpClient;\n+ Response: HttpResponseMessage;\n+ begin\n+ exit(Client.Get(ServiceUrl, Response));\n+ end;\n+}\n", "expected_comments": [{"file": "src/ElecVATSubmission.Codeunit.al", "line_start": 14, "line_end": 14, "domain": "security", "severity": "high", "body": "A caller-supplied URL is passed to HttpClient.Post without host or scheme validation, allowing SSRF requests to internal or attacker-chosen endpoints.", "article": "security/validate-user-configurable-urls"}, {"file": "src/ElecVATSubmission.Codeunit.al", "line_start": 22, "line_end": 22, "domain": "security", "severity": "high", "body": "A caller-supplied URL is passed to HttpClient.Get without host or scheme validation, allowing SSRF requests to internal or attacker-chosen endpoints.", "article": "security/validate-user-configurable-urls"}], "category": "code-review", "description": "True positive security findings: input_validation (trimmed to core input validation cases)", "expect_findings": true, "source": "vsoadmin"} -{"repo": "microsoft/BCApps", "instance_id": "synthetic__security-012", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/HttpAuthenticationBasic.Codeunit.al b/src/HttpAuthenticationBasic.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/HttpAuthenticationBasic.Codeunit.al\n@@ -0,0 +1,37 @@\n+codeunit 2359 \"Http Authentication Basic\"\n+{\n+ Access = Public;\n+ InherentEntitlements = X;\n+ InherentPermissions = X;\n+\n+ var\n+ Credential: SecretText;\n+ UsernameDomainTok: Label '%1\\%2', Comment = '%1 = domain, %2 = user name', Locked = true;\n+\n+ [NonDebuggable]\n+ procedure Initialize(Username: SecretText; Domain: Text; Password: SecretText)\n+ begin\n+ Credential := SecretStrSubstNo('%1:%2', QualifyUser(Username, Domain), Password);\n+ end;\n+\n+ procedure GetAuthorizationHeader() Header: SecretText\n+ begin\n+ Header := ToBase64(Credential);\n+ end;\n+\n+ [NonDebuggable]\n+ local procedure QualifyUser(Username: SecretText; Domain: Text): SecretText\n+ begin\n+ if Domain = '' then\n+ exit(Username);\n+ exit(SecretStrSubstNo(UsernameDomainTok, Domain, Username));\n+ end;\n+\n+ local procedure ToBase64(Value: SecretText) Base64Value: SecretText\n+ var\n+ Convert: DotNet Convert;\n+ Encoding: DotNet Encoding;\n+ begin\n+ Base64Value := Convert.ToBase64String(Encoding.UTF8().GetBytes(Value.Unwrap()));\n+ end;\n+}\n", "expected_comments": [{"file": "src/HttpAuthenticationBasic.Codeunit.al", "line_start": 30, "line_end": 30, "domain": "security", "severity": "medium", "body": "ToBase64 transforms SecretText credential material and calls Unwrap() without [NonDebuggable], so the plaintext credential is visible in the debugger.", "article": "security/nondebuggable-required-when-unwrapping-secrettext"}], "category": "code-review", "description": "True positive security findings: procedures handling passwords or SecretText values without [NonDebuggable]", "expect_findings": true, "source": "vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__security-008","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-05-15T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/PartnerConfigKeyManager.Codeunit.al b/src/PartnerConfigKeyManager.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/PartnerConfigKeyManager.Codeunit.al\n@@ -0,0 +1,9 @@\n+codeunit 50100 \"Partner Config Key Manager\"\n+{\n+ Access = Internal;\n+\n+ internal procedure StoreApiKey(KeyName: Text; ApiKey: Text)\n+ begin\n+ IsolatedStorage.SetEncrypted(KeyName, ApiKey, DataScope::Module);\n+ end;\n+}\n","expected_comments":[{"file":"src/PartnerConfigKeyManager.Codeunit.al","line_start":5,"line_end":5,"domain":"security","severity":"high","body":"The API key is accepted as a plain Text parameter instead of SecretText, so the secret is exposed in memory and to anyone inspecting the call stack or debugger.","articles":["security/secrettext-for-credentials"]}],"category":"code-review","description":"True positive security findings: encryption (trimmed to 5 representative findings)","expect_findings":true,"source":"vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__security-009","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-05-15T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/OutlookAddinDeployer.Codeunit.al b/src/OutlookAddinDeployer.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/OutlookAddinDeployer.Codeunit.al\n@@ -0,0 +1,37 @@\n+namespace Microsoft.Integration.Outlook;\n+\n+codeunit 50104 \"Outlook Addin Deployer\"\n+{\n+ Access = Internal;\n+\n+ var\n+ EndpointTok: Label 'https://outlook.office365.com/api/v2.0/addins/deploy', Locked = true;\n+ StatusErr: Label 'Deployment failed (HTTP %1): %2', Comment = '%1 is the HTTP status code, %2 is the raw response body.';\n+ ConnectErr: Label 'Failed to connect to the deployment service: %1', Comment = '%1 is the underlying error text.';\n+\n+ procedure DeployAddin(ManifestPath: Text)\n+ var\n+ Client: HttpClient;\n+ Content: HttpContent;\n+ Response: HttpResponseMessage;\n+ Headers: HttpHeaders;\n+ Payload: Text;\n+ ResponseText: Text;\n+ begin\n+ Payload := '{\"manifest\":\"' + ManifestPath + '\"}';\n+ Content.WriteFrom(Payload);\n+ Content.GetHeaders(Headers);\n+ Headers.Add('Authorization', 'Bearer ' + GetAccessToken());\n+ if Client.Post(EndpointTok, Content, Response) then begin\n+ Response.Content.ReadAs(ResponseText);\n+ if not Response.IsSuccessStatusCode() then\n+ Error(StatusErr, Response.HttpStatusCode(), ResponseText);\n+ end else\n+ Error(ConnectErr, GetLastErrorText());\n+ end;\n+\n+ local procedure GetAccessToken(): Text\n+ begin\n+ exit('dummy_access_token_for_testing');\n+ end;\n+}\n","expected_comments":[{"file":"src/OutlookAddinDeployer.Codeunit.al","line_start":21,"line_end":21,"domain":"security","severity":"medium","body":"The manifest path is concatenated directly into a JSON payload, allowing JSON injection. Build the payload with a JsonObject so values are escaped."},{"file":"src/OutlookAddinDeployer.Codeunit.al","line_start":28,"line_end":28,"domain":"security","severity":"medium","body":"The error surfaces the raw HTTP status code and full response body to the user, leaking internal service details. Log the details and show a generic message."},{"file":"src/OutlookAddinDeployer.Codeunit.al","line_start":30,"line_end":30,"domain":"security","severity":"medium","body":"GetLastErrorText() is shown to the user, exposing internal system details. Log the raw error and present a sanitized message."},{"file":"src/OutlookAddinDeployer.Codeunit.al","line_start":33,"line_end":33,"domain":"security","severity":"high","body":"The access token is returned as plain Text instead of SecretText, exposing it in memory and to the debugger. Return and handle it as SecretText.","articles":["security/secrettext-for-credentials"]},{"file":"src/OutlookAddinDeployer.Codeunit.al","line_start":35,"line_end":35,"domain":"security","severity":"high","body":"A hardcoded access token is embedded in source code. Retrieve the token from a secure store or OAuth flow instead of hardcoding it.","articles":["security/secrettext-for-credentials"]}],"category":"code-review","description":"True positive security findings: error_exposure (verified line numbers)","expect_findings":true,"source":"vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__security-011","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-05-15T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/ElecVATSubmission.Codeunit.al b/src/ElecVATSubmission.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/ElecVATSubmission.Codeunit.al\n@@ -0,0 +1,24 @@\n+namespace Microsoft.Finance.VAT;\n+\n+codeunit 13610 \"Elec VAT Submission\"\n+{\n+ Access = Internal;\n+\n+ procedure SubmitReturn(AuthorityUrl: Text): Boolean\n+ var\n+ Client: HttpClient;\n+ Content: HttpContent;\n+ Response: HttpResponseMessage;\n+ begin\n+ Content.WriteFrom('{}');\n+ exit(Client.Post(AuthorityUrl, Content, Response));\n+ end;\n+\n+ procedure CheckHealth(ServiceUrl: Text): Boolean\n+ var\n+ Client: HttpClient;\n+ Response: HttpResponseMessage;\n+ begin\n+ exit(Client.Get(ServiceUrl, Response));\n+ end;\n+}\n","expected_comments":[{"file":"src/ElecVATSubmission.Codeunit.al","line_start":14,"line_end":14,"domain":"security","severity":"high","body":"A caller-supplied URL is passed to HttpClient.Post without host or scheme validation, allowing SSRF requests to internal or attacker-chosen endpoints.","articles":["security/validate-user-configurable-urls"]},{"file":"src/ElecVATSubmission.Codeunit.al","line_start":22,"line_end":22,"domain":"security","severity":"high","body":"A caller-supplied URL is passed to HttpClient.Get without host or scheme validation, allowing SSRF requests to internal or attacker-chosen endpoints.","articles":["security/validate-user-configurable-urls"]}],"category":"code-review","description":"True positive security findings: input_validation (trimmed to core input validation cases)","expect_findings":true,"source":"vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__security-012","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-05-15T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/HttpAuthenticationBasic.Codeunit.al b/src/HttpAuthenticationBasic.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/HttpAuthenticationBasic.Codeunit.al\n@@ -0,0 +1,37 @@\n+codeunit 2359 \"Http Authentication Basic\"\n+{\n+ Access = Public;\n+ InherentEntitlements = X;\n+ InherentPermissions = X;\n+\n+ var\n+ Credential: SecretText;\n+ UsernameDomainTok: Label '%1\\%2', Comment = '%1 = domain, %2 = user name', Locked = true;\n+\n+ [NonDebuggable]\n+ procedure Initialize(Username: SecretText; Domain: Text; Password: SecretText)\n+ begin\n+ Credential := SecretStrSubstNo('%1:%2', QualifyUser(Username, Domain), Password);\n+ end;\n+\n+ procedure GetAuthorizationHeader() Header: SecretText\n+ begin\n+ Header := ToBase64(Credential);\n+ end;\n+\n+ [NonDebuggable]\n+ local procedure QualifyUser(Username: SecretText; Domain: Text): SecretText\n+ begin\n+ if Domain = '' then\n+ exit(Username);\n+ exit(SecretStrSubstNo(UsernameDomainTok, Domain, Username));\n+ end;\n+\n+ local procedure ToBase64(Value: SecretText) Base64Value: SecretText\n+ var\n+ Convert: DotNet Convert;\n+ Encoding: DotNet Encoding;\n+ begin\n+ Base64Value := Convert.ToBase64String(Encoding.UTF8().GetBytes(Value.Unwrap()));\n+ end;\n+}\n","expected_comments":[{"file":"src/HttpAuthenticationBasic.Codeunit.al","line_start":30,"line_end":30,"domain":"security","severity":"medium","body":"ToBase64 transforms SecretText credential material and calls Unwrap() without [NonDebuggable], so the plaintext credential is visible in the debugger.","articles":["security/nondebuggable-required-when-unwrapping-secrettext"]}],"category":"code-review","description":"True positive security findings: procedures handling passwords or SecretText values without [NonDebuggable]","expect_findings":true,"source":"vsoadmin"} {"repo": "microsoft/BCApps", "instance_id": "synthetic__security-013", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/ExpenseAgentAdmin.PermissionSet.al b/src/ExpenseAgentAdmin.PermissionSet.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/ExpenseAgentAdmin.PermissionSet.al\n@@ -0,0 +1,15 @@\n+// ------------------------------------------------------------------------------------------------\n+// Copyright (c) Microsoft Corporation. All rights reserved.\n+// Licensed under the MIT License. See License.txt in the project root for license information.\n+// ------------------------------------------------------------------------------------------------\n+namespace Microsoft.Agents.Expense;\n+\n+permissionset 50700 \"Expense Agent Admin\"\n+{\n+ Assignable = true;\n+ Caption = 'Expense Agent Administration';\n+\n+ Permissions =\n+ tabledata \"Agent Creation Control\" = RIMD,\n+ tabledata \"Expense Report Rule Violation\" = IMD;\n+}\ndiff --git a/src/ExpenseAgentConsumption.Table.al b/src/ExpenseAgentConsumption.Table.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/ExpenseAgentConsumption.Table.al\n@@ -0,0 +1,51 @@\n+// ------------------------------------------------------------------------------------------------\n+// Copyright (c) Microsoft Corporation. All rights reserved.\n+// Licensed under the MIT License. See License.txt in the project root for license information.\n+// ------------------------------------------------------------------------------------------------\n+namespace Microsoft.Agents.Expense;\n+\n+table 50600 \"Expense Agent Consumption\"\n+{\n+ Caption = 'Expense Agent Consumption';\n+ DataClassification = CustomerContent;\n+ InherentEntitlements = RIX;\n+ InherentPermissions = RIX;\n+\n+ fields\n+ {\n+ field(1; \"Entry No.\"; Integer)\n+ {\n+ Caption = 'Entry No.';\n+ DataClassification = SystemMetadata;\n+ AutoIncrement = true;\n+ }\n+ field(10; Amount; Decimal)\n+ {\n+ Caption = 'Amount';\n+ DataClassification = CustomerContent;\n+ }\n+ field(20; \"User Security ID\"; Guid)\n+ {\n+ Caption = 'User Security ID';\n+ DataClassification = EndUserPseudonymousIdentifiers;\n+ }\n+ }\n+\n+ keys\n+ {\n+ key(PK; \"Entry No.\")\n+ {\n+ Clustered = true;\n+ }\n+ }\n+\n+ procedure LogConsumption(CallerSecurityId: Guid; ConsumptionAmount: Decimal)\n+ var\n+ ConsumptionEntry: Record \"Expense Agent Consumption\";\n+ begin\n+ ConsumptionEntry.Init();\n+ ConsumptionEntry.\"User Security ID\" := CallerSecurityId;\n+ ConsumptionEntry.Amount := ConsumptionAmount;\n+ ConsumptionEntry.Insert();\n+ end;\n+}\n", "expected_comments": [{"file": "src/ExpenseAgentConsumption.Table.al", "line_start": 11, "line_end": 11, "domain": "security", "severity": "medium", "body": "InherentEntitlements and InherentPermissions of RIX grant read/insert/execute to every user regardless of assigned permission sets. Remove the inherent grants and control access explicitly."}, {"file": "src/ExpenseAgentConsumption.Table.al", "line_start": 42, "line_end": 42, "domain": "security", "severity": "medium", "body": "The procedure accepts an arbitrary UserSecurityId, letting a caller log consumption against any user's identity. Derive the user from UserSecurityId() instead of trusting the parameter."}, {"file": "src/ExpenseAgentAdmin.PermissionSet.al", "line_start": 13, "line_end": 13, "domain": "security", "severity": "medium", "body": "RIMD on Agent Creation Control lets assigned users delete creation-control records, removing a security guardrail. Grant only the permissions actually required."}, {"file": "src/ExpenseAgentAdmin.PermissionSet.al", "line_start": 14, "line_end": 14, "domain": "security", "severity": "medium", "body": "IMD on Expense Report Rule Violation lets users delete recorded policy violations, enabling them to hide their own violations. Remove delete and modify access."}], "category": "code-review", "description": "True positive security findings: permission (trimmed to 5 representative findings)", "expect_findings": true, "source": "vsoadmin"} -{"repo": "microsoft/BCApps", "instance_id": "synthetic__security-014", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/SecureOperationHelper.Codeunit.al b/src/SecureOperationHelper.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/SecureOperationHelper.Codeunit.al\n@@ -0,0 +1,13 @@\n+codeunit 50105 \"Secure Operation Helper\"\n+{\n+ Access = Internal;\n+\n+ internal procedure DeleteAllRecords(TableNo: Integer)\n+ var\n+ RecRef: RecordRef;\n+ begin\n+ RecRef.Open(TableNo);\n+ RecRef.DeleteAll();\n+ RecRef.Close();\n+ end;\n+}\n", "expected_comments": [{"file": "src/SecureOperationHelper.Codeunit.al", "line_start": 9, "line_end": 10, "domain": "security", "severity": "high", "body": "A caller-provided table number is opened with RecordRef.Open and then DeleteAll is called, letting any caller delete every record in an arbitrary table. Restrict the allowed tables and enforce permission checks.", "article": "security/recordref-open-with-caller-table-must-not-be-public"}], "category": "code-review", "description": "True positive: public procedure uses RecordRef.Open with caller-provided table number, allowing any extension to delete all records from any table through this codeunit's permissions", "expect_findings": true, "source": "vsoadmin"} -{"repo": "microsoft/BCApps", "instance_id": "synthetic__security-015", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/ExternalIntegrationMgt.Codeunit.al b/src/ExternalIntegrationMgt.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/ExternalIntegrationMgt.Codeunit.al\n@@ -0,0 +1,24 @@\n+namespace Microsoft.Integration.Partner;\n+\n+codeunit 50205 \"External Integration Mgt.\"\n+{\n+ Access = Internal;\n+\n+ procedure PostToPartner(EndpointUrl: Text): Boolean\n+ var\n+ Client: HttpClient;\n+ Content: HttpContent;\n+ Response: HttpResponseMessage;\n+ begin\n+ Content.WriteFrom('{}');\n+ exit(Client.Post(EndpointUrl, Content, Response));\n+ end;\n+\n+ procedure GetFromProvider(EndpointUrl: Text): Boolean\n+ var\n+ Client: HttpClient;\n+ Response: HttpResponseMessage;\n+ begin\n+ exit(Client.Get(EndpointUrl, Response));\n+ end;\n+}\n", "expected_comments": [{"file": "src/ExternalIntegrationMgt.Codeunit.al", "line_start": 14, "line_end": 14, "domain": "security", "severity": "high", "body": "A caller-supplied URL is passed to HttpClient.Post without host or scheme validation, allowing SSRF requests to internal or attacker-chosen endpoints.", "article": "security/validate-user-configurable-urls"}, {"file": "src/ExternalIntegrationMgt.Codeunit.al", "line_start": 22, "line_end": 22, "domain": "security", "severity": "high", "body": "A caller-supplied URL is passed to HttpClient.Get without host or scheme validation, allowing SSRF requests to internal or attacker-chosen endpoints.", "article": "security/validate-user-configurable-urls"}], "category": "code-review", "description": "True positive security findings: URLs from table fields used in HTTP requests without validation (SSRF risk). Three procedures use user-configurable URLs directly, while two procedures correctly validate using Uri.AreURIsHaveSameHost and Uri.IsValidURIPattern.", "expect_findings": true, "source": "vsoadmin"} -{"repo": "microsoft/BCApps", "instance_id": "synthetic__security-016", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "security"}, "patch": "diff --git a/src/ExpenseHtmlNotifier.Codeunit.al b/src/ExpenseHtmlNotifier.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/ExpenseHtmlNotifier.Codeunit.al\n@@ -0,0 +1,12 @@\n+codeunit 50900 \"Expense Html Notifier\"\n+{\n+ Access = Internal;\n+\n+ var\n+ BodyTemplateTok: Label '

Dear %1,

%2

', Locked = true;\n+\n+ internal procedure BuildNotificationBody(EmployeeName: Text; Description: Text): Text\n+ begin\n+ exit(StrSubstNo(BodyTemplateTok, EmployeeName, Description));\n+ end;\n+}\n", "expected_comments": [{"file": "src/ExpenseHtmlNotifier.Codeunit.al", "line_start": 10, "line_end": 10, "domain": "security", "severity": "high", "body": "User-supplied EmployeeName and Description are substituted into the HTML body without encoding, enabling stored or reflected XSS. HTML-encode the values before embedding them.", "article": "security/al-has-no-built-in-htmlencode"}], "category": "code-review", "description": "True positive security findings: xss (user-supplied data embedded in HTML without encoding)", "expect_findings": true, "source": "vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__security-014","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-05-15T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/SecureOperationHelper.Codeunit.al b/src/SecureOperationHelper.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/SecureOperationHelper.Codeunit.al\n@@ -0,0 +1,13 @@\n+codeunit 50105 \"Secure Operation Helper\"\n+{\n+ Access = Internal;\n+\n+ internal procedure DeleteAllRecords(TableNo: Integer)\n+ var\n+ RecRef: RecordRef;\n+ begin\n+ RecRef.Open(TableNo);\n+ RecRef.DeleteAll();\n+ RecRef.Close();\n+ end;\n+}\n","expected_comments":[{"file":"src/SecureOperationHelper.Codeunit.al","line_start":9,"line_end":10,"domain":"security","severity":"high","body":"A caller-provided table number is opened with RecordRef.Open and then DeleteAll is called, letting any caller delete every record in an arbitrary table. Restrict the allowed tables and enforce permission checks.","articles":["security/recordref-open-with-caller-table-must-not-be-public"]}],"category":"code-review","description":"True positive: public procedure uses RecordRef.Open with caller-provided table number, allowing any extension to delete all records from any table through this codeunit's permissions","expect_findings":true,"source":"vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__security-015","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-05-15T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/ExternalIntegrationMgt.Codeunit.al b/src/ExternalIntegrationMgt.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/ExternalIntegrationMgt.Codeunit.al\n@@ -0,0 +1,24 @@\n+namespace Microsoft.Integration.Partner;\n+\n+codeunit 50205 \"External Integration Mgt.\"\n+{\n+ Access = Internal;\n+\n+ procedure PostToPartner(EndpointUrl: Text): Boolean\n+ var\n+ Client: HttpClient;\n+ Content: HttpContent;\n+ Response: HttpResponseMessage;\n+ begin\n+ Content.WriteFrom('{}');\n+ exit(Client.Post(EndpointUrl, Content, Response));\n+ end;\n+\n+ procedure GetFromProvider(EndpointUrl: Text): Boolean\n+ var\n+ Client: HttpClient;\n+ Response: HttpResponseMessage;\n+ begin\n+ exit(Client.Get(EndpointUrl, Response));\n+ end;\n+}\n","expected_comments":[{"file":"src/ExternalIntegrationMgt.Codeunit.al","line_start":14,"line_end":14,"domain":"security","severity":"high","body":"A caller-supplied URL is passed to HttpClient.Post without host or scheme validation, allowing SSRF requests to internal or attacker-chosen endpoints.","articles":["security/validate-user-configurable-urls"]},{"file":"src/ExternalIntegrationMgt.Codeunit.al","line_start":22,"line_end":22,"domain":"security","severity":"high","body":"A caller-supplied URL is passed to HttpClient.Get without host or scheme validation, allowing SSRF requests to internal or attacker-chosen endpoints.","articles":["security/validate-user-configurable-urls"]}],"category":"code-review","description":"True positive security findings: URLs from table fields used in HTTP requests without validation (SSRF risk). Three procedures use user-configurable URLs directly, while two procedures correctly validate using Uri.AreURIsHaveSameHost and Uri.IsValidURIPattern.","expect_findings":true,"source":"vsoadmin"} +{"repo":"microsoft/BCApps","instance_id":"synthetic__security-016","base_commit":"70fd0246a0a4dbc72cb183ca719106722c03be4d","created_at":"2026-05-15T00:00:00Z","environment_setup_version":"27.0","project_paths":[],"metadata":{"area":"security"},"patch":"diff --git a/src/ExpenseHtmlNotifier.Codeunit.al b/src/ExpenseHtmlNotifier.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/ExpenseHtmlNotifier.Codeunit.al\n@@ -0,0 +1,12 @@\n+codeunit 50900 \"Expense Html Notifier\"\n+{\n+ Access = Internal;\n+\n+ var\n+ BodyTemplateTok: Label '

Dear %1,

%2

', Locked = true;\n+\n+ internal procedure BuildNotificationBody(EmployeeName: Text; Description: Text): Text\n+ begin\n+ exit(StrSubstNo(BodyTemplateTok, EmployeeName, Description));\n+ end;\n+}\n","expected_comments":[{"file":"src/ExpenseHtmlNotifier.Codeunit.al","line_start":10,"line_end":10,"domain":"security","severity":"high","body":"User-supplied EmployeeName and Description are substituted into the HTML body without encoding, enabling stored or reflected XSS. HTML-encode the values before embedding them.","articles":["security/al-has-no-built-in-htmlencode"]}],"category":"code-review","description":"True positive security findings: xss (user-supplied data embedded in HTML without encoding)","expect_findings":true,"source":"vsoadmin"} {"repo": "microsoft/BCApps", "instance_id": "synthetic__performance-001", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "performance"}, "patch": "diff --git a/src/FADepreciationBook.Table.al b/src/FADepreciationBook.Table.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/FADepreciationBook.Table.al\n@@ -0,0 +1,78 @@\n+table 50200 \"FA Depreciation Book FP\"\n+{\n+ DataClassification = CustomerContent;\n+\n+ fields\n+ {\n+ field(1; \"FA No.\"; Code[20])\n+ {\n+ Caption = 'FA No.';\n+ TableRelation = \"Fixed Asset\";\n+ }\n+\n+ field(2; \"Depreciation Book Code\"; Code[10])\n+ {\n+ Caption = 'Depreciation Book Code';\n+ TableRelation = \"Depreciation Book\";\n+ }\n+\n+ field(3; Depreciation; Decimal)\n+ {\n+ FieldClass = FlowField;\n+ CalcFormula = sum(\"FA Ledger Entry\".Amount where(\"FA No.\" = field(\"FA No.\"),\n+ \"Depreciation Book Code\" = field(\"Depreciation Book Code\"),\n+ \"FA Posting Category\" = const(Depreciation)));\n+ Caption = 'Depreciation';\n+ }\n+\n+ field(4; \"Bonus Depr. Applied Amount\"; Decimal)\n+ {\n+ FieldClass = FlowField;\n+ CalcFormula = sum(\"FA Ledger Entry\".Amount where(\"FA No.\" = field(\"FA No.\"),\n+ \"Depreciation Book Code\" = field(\"Depreciation Book Code\"),\n+ \"FA Posting Type\" = const(\"Bonus Depreciation\")));\n+ Caption = 'Bonus Depr. Applied Amount';\n+ }\n+\n+ field(5; \"Use Half-Year Convention\"; Boolean)\n+ {\n+ Caption = 'Use Half-Year Convention';\n+\n+ trigger OnValidate()\n+ var\n+ CannotChangeHalfYearErr: Label 'Cannot change half-year convention after depreciation has been posted.';\n+ CannotChangeBonusErr: Label 'Cannot change half-year convention when bonus depreciation has been applied.';\n+ begin\n+ // CORRECT: CalcFields in OnValidate runs once per user edit, not in a loop\n+ // This is appropriate for validation logic that needs current flowfield values\n+ CalcFields(Depreciation);\n+ if Depreciation <> 0 then\n+ Error(CannotChangeHalfYearErr);\n+\n+ CalcFields(\"Bonus Depr. Applied Amount\");\n+ if \"Bonus Depr. Applied Amount\" <> 0 then\n+ Error(CannotChangeBonusErr);\n+ end;\n+ }\n+\n+ field(6; \"Depreciation Method\"; Option)\n+ {\n+ OptionCaption = 'Straight-Line,Declining-Balance 1,Declining-Balance 2';\n+ OptionMembers = \"Straight-Line\",\"Declining-Balance 1\",\"Declining-Balance 2\";\n+ Caption = 'Depreciation Method';\n+ }\n+\n+ field(7; \"Starting Date\"; Date)\n+ {\n+ Caption = 'Depreciation Starting Date';\n+ }\n+ }\n+\n+ keys\n+ {\n+ key(PK; \"FA No.\", \"Depreciation Book Code\")\n+ {\n+ Clustered = true;\n+ }\n+ }\n+}\ndiff --git a/src/SalesOrderCard.Page.al b/src/SalesOrderCard.Page.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/SalesOrderCard.Page.al\n@@ -0,0 +1,87 @@\n+page 50201 \"Sales Order Card FP\"\n+{\n+ PageType = Card;\n+ SourceTable = \"Sales Header\";\n+ Caption = 'Sales Order Card FP';\n+\n+ layout\n+ {\n+ area(Content)\n+ {\n+ group(General)\n+ {\n+ Caption = 'General';\n+\n+ field(\"No.\"; Rec.\"No.\")\n+ {\n+ ApplicationArea = All;\n+ ToolTip = 'Specifies the number of the sales order.';\n+ }\n+\n+ field(\"Sell-to Customer No.\"; Rec.\"Sell-to Customer No.\")\n+ {\n+ ApplicationArea = All;\n+ ToolTip = 'Specifies the number of the customer who will receive the products on the sales order.';\n+ }\n+\n+ field(\"Document Date\"; Rec.\"Document Date\")\n+ {\n+ ApplicationArea = All;\n+ ToolTip = 'Specifies the date when the sales order was created.';\n+ }\n+\n+ field(\"Total Amount\"; TotalAmount)\n+ {\n+ Caption = 'Total Amount Including VAT';\n+ ApplicationArea = All;\n+ Editable = false;\n+ ToolTip = 'Specifies the total amount including VAT for the sales order.';\n+ }\n+ }\n+ }\n+ }\n+\n+ actions\n+ {\n+ area(Processing)\n+ {\n+ action(RefreshTotals)\n+ {\n+ Caption = 'Refresh Totals';\n+ ApplicationArea = All;\n+ ToolTip = 'Recalculates and refreshes the total amount for the sales order.';\n+ Image = Refresh;\n+\n+ trigger OnAction()\n+ var\n+ TotalRefreshedMsg: Label 'Total refreshed: %1', Comment = '%1 = total amount including VAT';\n+ begin\n+ // CORRECT: Manual refresh action - user-initiated, runs once\n+ Rec.CalcFields(\"Amount Including VAT\");\n+ TotalAmount := Rec.\"Amount Including VAT\";\n+ Message(TotalRefreshedMsg, TotalAmount);\n+ end;\n+ }\n+ }\n+ }\n+\n+ var\n+ TotalAmount: Decimal;\n+\n+ // CORRECT: OnAfterGetCurrRecord fires once per record selection, not per row\n+ // This is the appropriate place to calculate values when user navigates to a record\n+ trigger OnAfterGetCurrRecord()\n+ begin\n+ // Calculate total amount when user selects a sales order\n+ // This runs once when the record is loaded/selected, not in a loop\n+ Rec.CalcFields(\"Amount Including VAT\");\n+ TotalAmount := Rec.\"Amount Including VAT\";\n+ end;\n+\n+ trigger OnNewRecord(BelowxRec: Boolean)\n+ begin\n+ // CORRECT: Initialize values for new record - runs once per new record creation\n+ TotalAmount := 0;\n+ Rec.\"Document Date\" := WorkDate();\n+ end;\n+}\ndiff --git a/src/CustLedgerEntryAggregator.Codeunit.al b/src/CustLedgerEntryAggregator.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/CustLedgerEntryAggregator.Codeunit.al\n@@ -0,0 +1,20 @@\n+codeunit 50202 \"Cust Ledger Entry Aggregator\"\n+{\n+ procedure SumOpenRemainingAmount(CustomerNo: Code[20]): Decimal\n+ var\n+ CustLedgerEntry: Record \"Cust. Ledger Entry\";\n+ Customer: Record Customer;\n+ Total: Decimal;\n+ begin\n+ CustLedgerEntry.SetRange(\"Customer No.\", CustomerNo);\n+ CustLedgerEntry.SetRange(Open, true);\n+ if CustLedgerEntry.FindSet() then\n+ repeat\n+ CustLedgerEntry.CalcFields(\"Remaining Amount\");\n+ Customer.Get(CustLedgerEntry.\"Customer No.\");\n+ if Customer.\"Application Method\" = Customer.\"Application Method\"::Manual then\n+ Total += CustLedgerEntry.\"Remaining Amount\";\n+ until CustLedgerEntry.Next() = 0;\n+ exit(Total);\n+ end;\n+}\n", "expected_comments": [{"file": "src/CustLedgerEntryAggregator.Codeunit.al", "line_start": 13, "line_end": 13, "body": "CalcFields(\"Remaining Amount\") inside a repeat..until loop over \"Cust. Ledger Entry\" (up to 10M rows) issues one SQL query per iteration — classic N+1 against a hot table. — Replace the loop with `CustLedgerEntry.CalcSums(\"Remaining Amount\")` which executes as a single SUM query, or use a SIFT-backed key.", "severity": "high", "domain": "performance"}, {"file": "src/CustLedgerEntryAggregator.Codeunit.al", "line_start": 14, "line_end": 14, "body": "Customer.Get(CustLedgerEntry.\"Customer No.\") inside a repeat..until over Cust. Ledger Entry is an N+1 query: one Customer lookup per ledger row, redundant since every row already has the same Customer No. (the loop is filtered by CustomerNo). — Move the Customer.Get above the loop (single lookup), and add `Customer.SetLoadFields(\"Application Method\")` since only one field is read.", "severity": "high", "domain": "performance"}], "category": "code-review", "description": "False positive performance findings: calcfields_false_positive (30 false positives). Agent flagged these but reviewers rejected them. Enriched with 2 true-positive findings in addition to the false-positive bait.", "expect_findings": true, "source": "vsoadmin"} {"repo": "microsoft/BCApps", "instance_id": "synthetic__performance-002", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "performance"}, "patch": "diff --git a/src/SetupReader.Codeunit.al b/src/SetupReader.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/SetupReader.Codeunit.al\n@@ -0,0 +1,67 @@\n+codeunit 50211 \"Setup Reader\"\n+{\n+ procedure GetSetupValues()\n+ var\n+ GLSetup: Record \"General Ledger Setup\";\n+ SalesSetup: Record \"Sales & Receivables Setup\";\n+ InventorySetup: Record \"Inventory Setup\";\n+ PurchSetup: Record \"Purchases & Payables Setup\";\n+ begin\n+ // CORRECT: Setup tables typically have only 1 record per company\n+ // Any access pattern (Get, FindSet, FindFirst) is fine for singleton tables\n+ GLSetup.Get();\n+ SalesSetup.Get();\n+ InventorySetup.Get();\n+ PurchSetup.Get();\n+\n+ if GLSetup.\"Additional Reporting Currency\" <> '' then\n+ ProcessACYSettings(GLSetup);\n+\n+ if SalesSetup.\"Credit Warnings\" <> SalesSetup.\"Credit Warnings\"::\"No Warning\" then\n+ EnableCreditWarnings(SalesSetup);\n+ end;\n+\n+ procedure ValidateCompanySettings(): Boolean\n+ var\n+ CompanyInfo: Record \"Company Information\";\n+ begin\n+ // CORRECT: Company Information is a singleton table (1 record per company)\n+ // Get() is the appropriate method for singleton tables\n+ if not CompanyInfo.Get() then\n+ exit(false);\n+\n+ if CompanyInfo.Name = '' then\n+ exit(false);\n+\n+ if CompanyInfo.\"Country/Region Code\" = '' then\n+ exit(false);\n+\n+ exit(true);\n+ end;\n+\n+ procedure GetUserSetupForCurrentUser(var UserSetup: Record \"User Setup\"): Boolean\n+ begin\n+ // CORRECT: Looking up single user's setup record\n+ // Get() with UserId is appropriate for single-record lookup\n+ UserSetup.Reset();\n+ if UserSetup.Get(UserId) then\n+ exit(true);\n+ exit(false);\n+ end;\n+\n+ local procedure ProcessACYSettings(GLSetup: Record \"General Ledger Setup\")\n+ var\n+ ACYEnabledMsg: Label 'ACY is enabled: %1', Comment = '%1 = additional reporting currency';\n+ begin\n+ // Process additional currency settings\n+ Message(ACYEnabledMsg, GLSetup.\"Additional Reporting Currency\");\n+ end;\n+\n+ local procedure EnableCreditWarnings(SalesSetup: Record \"Sales & Receivables Setup\")\n+ var\n+ CreditWarningsEnabledMsg: Label 'Credit warnings are enabled';\n+ begin\n+ // Enable credit warning processing\n+ Message(CreditWarningsEnabledMsg);\n+ end;\n+}\ndiff --git a/src/TempBufferProcessor.Codeunit.al b/src/TempBufferProcessor.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/TempBufferProcessor.Codeunit.al\n@@ -0,0 +1,61 @@\n+codeunit 50210 \"Temp Buffer Processor\"\n+{\n+ procedure ProcessBufferEntries(var TempBuffer: Record \"Integer\" temporary)\n+ var\n+ ProcessedCount: Integer;\n+ TotalAmount: Decimal;\n+ ProcessedEntriesMsg: Label 'Processed %1 entries with total %2', Comment = '%1 = number of entries, %2 = total amount';\n+ begin\n+ // CORRECT: TempBuffer is temporary — all operations are in-memory, no SQL queries\n+ // Any access pattern (FindSet, Get, loops) on temp tables is performant\n+ ProcessedCount := 0;\n+ TotalAmount := 0;\n+\n+ if TempBuffer.FindSet() then\n+ repeat\n+ // This might look suspicious, but it's CORRECT because:\n+ // 1. TempBuffer is temporary (in-memory)\n+ // 2. No database round trips are happening\n+ // 3. All data is already loaded in memory\n+ TotalAmount += TempBuffer.Number;\n+ ProcessedCount += 1;\n+\n+ // Even modifying temp records in a loop is fine\n+ TempBuffer.Number := TempBuffer.Number * 2;\n+ TempBuffer.Modify();\n+\n+ until TempBuffer.Next() = 0;\n+\n+ Message(ProcessedEntriesMsg, ProcessedCount, TotalAmount);\n+ end;\n+\n+ procedure BuildTempData(var TempBuffer: Record \"Integer\" temporary)\n+ var\n+ i: Integer;\n+ begin\n+ // CORRECT: Building temp data - all operations are in-memory\n+ TempBuffer.Reset();\n+ TempBuffer.DeleteAll();\n+\n+ for i := 1 to 100 do begin\n+ TempBuffer.Init();\n+ TempBuffer.Number := Random(1000);\n+ TempBuffer.Insert();\n+ end;\n+ end;\n+\n+ procedure FindMaxValue(var TempBuffer: Record \"Integer\" temporary): Integer\n+ var\n+ MaxValue: Integer;\n+ begin\n+ // CORRECT: Finding max in temp table - no performance concern\n+ MaxValue := 0;\n+ if TempBuffer.FindSet() then\n+ repeat\n+ if TempBuffer.Number > MaxValue then\n+ MaxValue := TempBuffer.Number;\n+ until TempBuffer.Next() = 0;\n+\n+ exit(MaxValue);\n+ end;\n+}\ndiff --git a/src/CustomerLookup.Codeunit.al b/src/CustomerLookup.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/CustomerLookup.Codeunit.al\n@@ -0,0 +1,20 @@\n+codeunit 50212 \"Customer Lookup\"\n+{\n+ procedure GetCustomerName(CustomerNo: Code[20]): Text[100]\n+ var\n+ Customer: Record Customer;\n+ begin\n+ Customer.SetRange(\"No.\", CustomerNo);\n+ if Customer.FindFirst() then\n+ exit(Customer.Name);\n+ exit('');\n+ end;\n+\n+ procedure HasCustomersInCountry(CountryRegionCode: Code[10]): Boolean\n+ var\n+ Customer: Record Customer;\n+ begin\n+ Customer.SetRange(\"Country/Region Code\", CountryRegionCode);\n+ exit(Customer.Count() > 0);\n+ end;\n+}\n", "expected_comments": [{"file": "src/CustomerLookup.Codeunit.al", "line_start": 8, "line_end": 8, "body": "FindFirst() after SetRange on the full primary key (\"No.\") of Customer (up to 800k rows). This still does a SQL SELECT TOP 1 with a range predicate instead of a direct key lookup. — Replace with `if Customer.Get(CustomerNo) then exit(Customer.Name);` which is a direct PK lookup (CodeCop AA0233).", "severity": "medium", "domain": "performance"}, {"file": "src/CustomerLookup.Codeunit.al", "line_start": 18, "line_end": 18, "body": "Count() > 0 on Customer (up to 800k rows) for a pure existence check. Count() materializes a SQL COUNT(*) over the filtered set instead of stopping at the first matching row. — Replace with `exit(not Customer.IsEmpty());` which stops at the first match and is significantly cheaper on large tables.", "severity": "medium", "domain": "performance"}], "category": "code-review", "description": "False positive performance findings: findset_false_positive (69 false positives). Agent flagged these but reviewers rejected them. Enriched with 2 true-positive findings in addition to the false-positive bait.", "expect_findings": true, "source": "vsoadmin"} {"repo": "microsoft/BCApps", "instance_id": "synthetic__performance-003", "base_commit": "70fd0246a0a4dbc72cb183ca719106722c03be4d", "created_at": "2026-05-15T00:00:00Z", "environment_setup_version": "27.0", "project_paths": [], "metadata": {"area": "performance"}, "patch": "diff --git a/src/MigrationSetupHandler.Codeunit.al b/src/MigrationSetupHandler.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/MigrationSetupHandler.Codeunit.al\n@@ -0,0 +1,18 @@\n+codeunit 50221 \"Migration Setup Handler\"\n+{\n+ procedure CountMigratablePermissionSets(): Integer\n+ var\n+ PermissionSet: Record \"Permission Set\";\n+ begin\n+ PermissionSet.SetFilter(\"Role ID\", '%1|%2', 'D365 BASIC', 'D365 READ');\n+ exit(PermissionSet.Count());\n+ end;\n+\n+ procedure CountObsoleteRegisters(): Integer\n+ var\n+ DateComprRegister: Record \"Date Compr. Register\";\n+ begin\n+ DateComprRegister.SetFilter(\"Ending Date\", '<%1', CalcDate('<-2Y>', Today));\n+ exit(DateComprRegister.Count());\n+ end;\n+}\ndiff --git a/src/PermissionSetListOverview.Page.al b/src/PermissionSetListOverview.Page.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/PermissionSetListOverview.Page.al\n@@ -0,0 +1,84 @@\n+page 50220 \"Permission Set List Overview\"\n+{\n+ PageType = List;\n+ ApplicationArea = All;\n+ UsageCategory = Administration;\n+ SourceTable = \"Aggregate Permission Set\";\n+ Caption = 'Permission Set List Overview';\n+ Editable = false;\n+\n+ layout\n+ {\n+ area(Content)\n+ {\n+ repeater(Permissions)\n+ {\n+ field(\"Role ID\"; Rec.\"Role ID\")\n+ {\n+ ApplicationArea = All;\n+ ToolTip = 'Specifies the identifier of the permission set.';\n+ }\n+\n+ field(Name; Rec.Name)\n+ {\n+ ApplicationArea = All;\n+ ToolTip = 'Specifies the display name of the permission set.';\n+ }\n+\n+ field(Scope; Rec.Scope)\n+ {\n+ ApplicationArea = All;\n+ ToolTip = 'Specifies whether the permission set is defined by the system or by a tenant.';\n+ }\n+\n+ field(\"App Name\"; Rec.\"App Name\")\n+ {\n+ ApplicationArea = All;\n+ ToolTip = 'Specifies the name of the extension that defines the permission set.';\n+ }\n+\n+ field(\"Permission Count\"; PermissionCount)\n+ {\n+ Caption = 'Permission Count';\n+ ApplicationArea = All;\n+ Editable = false;\n+ ToolTip = 'Specifies the number of permissions that belong to the permission set.';\n+ }\n+ }\n+ }\n+ }\n+\n+ actions\n+ {\n+ area(Processing)\n+ {\n+ action(RefreshCounts)\n+ {\n+ Caption = 'Refresh Permission Counts';\n+ ApplicationArea = All;\n+ ToolTip = 'Recalculates the permission count shown for each permission set.';\n+\n+ trigger OnAction()\n+ begin\n+ CurrPage.Update();\n+ end;\n+ }\n+ }\n+ }\n+\n+ var\n+ PermissionCount: Integer;\n+\n+ trigger OnAfterGetRecord()\n+ var\n+ Permission: Record Permission;\n+ begin\n+ Permission.SetRange(\"Role ID\", Rec.\"Role ID\");\n+ PermissionCount := Permission.Count();\n+ end;\n+\n+ trigger OnOpenPage()\n+ begin\n+ Rec.SetFilter(Scope, '%1|%2', Rec.Scope::System, Rec.Scope::Tenant);\n+ end;\n+}\ndiff --git a/src/SalesInvoiceFilter.Codeunit.al b/src/SalesInvoiceFilter.Codeunit.al\nnew file mode 100644\n--- /dev/null\n+++ b/src/SalesInvoiceFilter.Codeunit.al\n@@ -0,0 +1,26 @@\n+codeunit 50222 \"Sales Invoice Filter\"\n+{\n+ procedure ListLinesByDescription(Description: Text[100])\n+ var\n+ SalesInvoiceLine: Record \"Sales Invoice Line\";\n+ begin\n+ SalesInvoiceLine.SetRange(Description, Description);\n+ if SalesInvoiceLine.FindSet() then\n+ repeat\n+ Message('%1 %2', SalesInvoiceLine.\"Document No.\", SalesInvoiceLine.\"Line No.\");\n+ until SalesInvoiceLine.Next() = 0;\n+ end;\n+\n+ procedure SumQuantityByDocument(DocumentNo: Code[20]): Decimal\n+ var\n+ SalesInvoiceLine: Record \"Sales Invoice Line\";\n+ Total: Decimal;\n+ begin\n+ SalesInvoiceLine.SetRange(\"Document No.\", DocumentNo);\n+ if SalesInvoiceLine.FindSet() then\n+ repeat\n+ Total += SalesInvoiceLine.Quantity;\n+ until SalesInvoiceLine.Next() = 0;\n+ exit(Total);\n+ end;\n+}\n", "expected_comments": [{"file": "src/SalesInvoiceFilter.Codeunit.al", "line_start": 7, "line_end": 7, "body": "SetRange on Sales Invoice Line.Description with no SetCurrentKey and no key including Description. Sales Invoice Line is large (up to 3M rows) and the query will table-scan. — Either add `SetCurrentKey` to a key whose leading field matches the filter, or introduce a new key on the source table that covers Description, before filtering.", "severity": "high", "domain": "performance"}, {"file": "src/SalesInvoiceFilter.Codeunit.al", "line_start": 20, "line_end": 20, "body": "FindSet over Sales Invoice Line (3M rows, ~80 fields) loads every field for every row but the loop only reads `Quantity`. — Add `SalesInvoiceLine.SetLoadFields(Quantity);` before SetRange so SQL returns only the Quantity column (plus key fields). Even better, replace the loop with `SalesInvoiceLine.CalcSums(Quantity)` since Quantity is a SumIndexField on this table.", "severity": "medium", "domain": "performance"}, {"file": "src/PermissionSetListOverview.Page.al", "line_start": 77, "line_end": 77, "body": "PermissionCount is computed with Permission.Count() inside OnAfterGetRecord, which fires once per row rendered on this List page (and again while scrolling), so every visible row issues its own SQL COUNT against the Permission table (a per-row N+1 that scales with the number of permission sets shown). — Expose the value as a FlowField on the source table (FieldClass = FlowField, CalcFormula = count(Permission where(\"Role ID\" = field(\"Role ID\")))) so the aggregate is computed by the query engine instead of recomputing it per row.", "severity": "high", "domain": "performance"}], "category": "code-review", "description": "False positive performance findings: index_false_positive (29 false positives). Agent flagged these but reviewers rejected them. Enriched with 2 true-positive findings in addition to the false-positive bait.", "expect_findings": true, "source": "vsoadmin"} diff --git a/notebooks/code-review-coverage.ipynb b/notebooks/code-review-coverage.ipynb index 08ca098d5..ec5cf92fe 100644 --- a/notebooks/code-review-coverage.ipynb +++ b/notebooks/code-review-coverage.ipynb @@ -8,8 +8,8 @@ "# Per-article BCQuality coverage (code-review)\n", "\n", "Ad-hoc analysis of how the code-review gold dataset maps onto BCQuality\n", - "knowledge articles. Every finding is annotated with the article it derives\n", - "from (`ReviewComment.article`), and false-positive-guard entries carry their\n", + "knowledge articles. Every finding is annotated with the articles it derives\n", + "from (`ReviewComment.articles`), and false-positive-guard entries carry their\n", "association at entry level (`metadata.articles`). This notebook aggregates\n", "those annotations via `bcbench.analysis.bcquality_article_coverage`.\n", "\n", @@ -28,7 +28,7 @@ "name": "stdout", "output_type": "stream", "text": [ - "9 articles covered across 17/130 annotated entries; 241 of 250 articles have zero coverage\n" + "9 articles covered across 17/144 annotated entries; 241 of 250 articles have zero coverage\n" ] } ], @@ -408,25 +408,36 @@ " - web-services/version-apis-by-adding-not-mutating-published-versions\n", " - web-services/webhook-eligibility-and-validationtoken-renewal\n", "\n", - "Unannotated entries (113):\n", + "Unannotated entries (127):\n", + " - synthetic__accessibility-setselectionfilter-scope-01\n", " - synthetic__breaking-access-modifier-01\n", + " - synthetic__breaking-event-subscriber-suppress-01\n", " - synthetic__breaking-notification-callback-01\n", + " - synthetic__breaking-protected-var-field-01\n", " - synthetic__breaking-relocation-01\n", + " - synthetic__breaking-scope-creep-unrelated-api-01\n", " - synthetic__caption-clean-01\n", " - synthetic__currrec-clean-01\n", " - synthetic__data-modeling-blocked-validation-skip-01\n", " - synthetic__data-modeling-excluded-from-calculation-01\n", + " - synthetic__data-modeling-tablerelation-restriction-mismatch-01\n", " - synthetic__errh-errortype-internal-01\n", " - synthetic__errh-tryfunction-swallowed-01\n", " - synthetic__error-clean-01\n", + " - synthetic__error-handling-assistedit-cancel-01\n", + " - synthetic__error-handling-case-unreachable-else-01\n", + " - synthetic__error-handling-drilldown-position-01\n", + " - synthetic__error-handling-errorinfo-actionable-boundary-01\n", " - synthetic__error-handling-fieldno-swap-01\n", " - synthetic__error-handling-guiallowed-01\n", " - synthetic__error-handling-silent-skip-01\n", " - synthetic__error-testfield-enabled-01\n", + " - synthetic__events-ishandled-reset-boundary-01\n", " - synthetic__get-clean-01\n", " - synthetic__obsolete-clean-01\n", " - synthetic__perf-batched-commit-checkpoint-01\n", " - synthetic__perf-clean-01\n", + " - synthetic__perf-progress-dialog-deleteall-01\n", " - synthetic__performance-001\n", " - synthetic__performance-002\n", " - synthetic__performance-003\n", @@ -502,9 +513,12 @@ " - synthetic__style-clean-03\n", " - synthetic__style-clean-04\n", " - synthetic__style-duplicate-action-01\n", + " - synthetic__style-field-repurpose-indent-01\n", + " - synthetic__style-showmandatory-flowfield-01\n", " - synthetic__style-this-keyword-01\n", " - synthetic__style-tooltip-mismatch-01\n", " - synthetic__testing-tolerance-clean-01\n", + " - synthetic__testing-ui-handler-assert-after-run-01\n", " - synthetic__upgrade-001\n", " - synthetic__upgrade-002\n", " - synthetic__upgrade-003\n", diff --git a/src/bcbench/dataset/codereview.py b/src/bcbench/dataset/codereview.py index 4281e4f14..55a6242f4 100644 --- a/src/bcbench/dataset/codereview.py +++ b/src/bcbench/dataset/codereview.py @@ -53,7 +53,9 @@ def from_input(cls, value: str) -> Severity: class ReviewComment(BaseModel): - model_config = ConfigDict(frozen=True) + # Reject unknown keys so a superseded annotation (e.g. the singular `article` this + # model used before) fails loudly instead of being dropped into invisible coverage loss. + model_config = ConfigDict(frozen=True, extra="forbid") file: Annotated[str, Field(pattern=r"^[A-Za-z0-9][A-Za-z0-9 ./_-]*\.(al|json)$")] line_start: Annotated[int, Field(ge=1)] @@ -61,10 +63,12 @@ class ReviewComment(BaseModel): domain: str | None = None body: Annotated[str, Field(min_length=1)] severity: Severity | None = None - # BCQuality knowledge article this finding derives from, as `/` - # (e.g. `security/hardcoded-secret`). Optional and backward-compatible; drives - # per-article coverage tracking. Older entries leave it unset (counted as unannotated). - article: ArticleId | None = None + # BCQuality knowledge articles this finding derives from, as `/` + # (e.g. `security/hardcoded-secret`). A single finding can exercise several articles, + # most commonly when it pairs a positive requirement with an explicit false-positive + # boundary. Optional; drives per-article coverage tracking. Entries that leave it + # empty are counted as unannotated. + articles: list[ArticleId] = Field(default_factory=list) @field_validator("severity", mode="before") @classmethod @@ -89,7 +93,7 @@ class CodeReviewEntryMetadata(EntryMetadata): # BCQuality knowledge articles this entry exercises as `/`. Primarily # for false-positive-guard entries (expected_comments=[]) that test an article by - # omission and thus have no per-comment `article` to carry the association. + # omission and thus have no per-comment `articles` to carry the association. articles: list[ArticleId] = Field(default_factory=list) @@ -115,7 +119,7 @@ def _validate_article_annotations(self) -> Self: empty). An article already declared on a comment must not be repeated at entry level, so the two annotation sources cannot silently drift apart. """ - comment_articles = {c.article for c in self.expected_comments if c.article} + comment_articles = {article for c in self.expected_comments for article in c.articles} overlap = comment_articles & set(self.metadata.articles) if overlap: raise ValueError( @@ -134,10 +138,10 @@ def get_expected_output(self) -> str: def declared_articles(self) -> set[ArticleId]: """BCQuality articles this entry is annotated against. - Union of every expected comment's `article` and the entry-level + Union of every expected comment's `articles` and the entry-level `metadata.articles` (which carries the association for false-positive-guard entries whose `expected_comments` is empty). """ - articles = {c.article for c in self.expected_comments if c.article} + articles = {article for c in self.expected_comments for article in c.articles} articles.update(self.metadata.articles) return articles diff --git a/tests/test_bcquality_article_coverage.py b/tests/test_bcquality_article_coverage.py index b0035d38d..4537dad35 100644 --- a/tests/test_bcquality_article_coverage.py +++ b/tests/test_bcquality_article_coverage.py @@ -35,15 +35,15 @@ def _entry( ) -def _comment(body: str, *, article: ArticleId | None = None) -> ReviewComment: - return ReviewComment(file="src/A.al", line_start=1, body=body, article=article) +def _comment(body: str, *articles: ArticleId) -> ReviewComment: + return ReviewComment(file="src/A.al", line_start=1, body=body, articles=list(articles)) class TestDeclaredArticles: def test_declared_from_comment_and_metadata(self): entry = _entry( "synthetic__security-001", - comments=[_comment("finding", article="security/secrettext-for-credentials")], + comments=[_comment("finding", "security/secrettext-for-credentials")], articles=["security/permission-set-avoid-wildcard-grants"], ) assert entry.declared_articles() == { @@ -51,6 +51,26 @@ def test_declared_from_comment_and_metadata(self): "security/permission-set-avoid-wildcard-grants", } + def test_comment_may_declare_several_articles(self): + entry = _entry( + "synthetic__error-handling-errorinfo-actionable-boundary-01", + comments=[ + _comment( + "raise ErrorInfo with a navigation action; do not require a Fix-it here", + "error-handling/prefer-errorinfo-for-actionable-errors", + "error-handling/fielderror-vs-testfield", + ) + ], + ) + assert entry.declared_articles() == { + "error-handling/prefer-errorinfo-for-actionable-errors", + "error-handling/fielderror-vs-testfield", + } + + def test_superseded_singular_article_key_is_rejected(self): + with pytest.raises(ValidationError, match="article"): + ReviewComment(file="src/A.al", line_start=1, body="x", article="security/secrettext-for-credentials") # ty: ignore[unknown-argument] + def test_unannotated_entry_declares_nothing(self): entry = _entry("synthetic__security-002", comments=[_comment("finding")]) assert entry.declared_articles() == set() @@ -60,15 +80,15 @@ def test_article_declared_both_places_is_rejected(self): with pytest.raises(ValidationError, match="declared both per-comment"): _entry( "synthetic__security-003", - comments=[_comment("finding", article=shared)], + comments=[_comment("finding", shared)], articles=[shared], ) def test_collect_maps_article_to_sorted_entry_ids(self): shared = "security/validate-user-configurable-urls" entries = [ - _entry("synthetic__security-015", comments=[_comment("ssrf", article=shared)]), - _entry("synthetic__security-011", comments=[_comment("ssrf", article=shared)]), + _entry("synthetic__security-015", comments=[_comment("ssrf", shared)]), + _entry("synthetic__security-011", comments=[_comment("ssrf", shared)]), ] declared = collect_declared_articles(entries) assert declared[shared] == ["synthetic__security-011", "synthetic__security-015"] @@ -77,7 +97,7 @@ def test_collect_maps_article_to_sorted_entry_ids(self): class TestCoverageReport: def test_without_inventory_reports_declared_only(self): entries = [ - _entry("synthetic__security-001", comments=[_comment("x", article="security/secrettext-for-credentials")]), + _entry("synthetic__security-001", comments=[_comment("x", "security/secrettext-for-credentials")]), _entry("synthetic__security-002", comments=[_comment("y")]), ] report = build_coverage_report(entries, inventory=None) @@ -93,8 +113,8 @@ def test_with_inventory_flags_zero_and_unknown(self): "security/permission-set-avoid-wildcard-grants", } entries = [ - _entry("synthetic__security-001", comments=[_comment("x", article="security/secrettext-for-credentials")]), - _entry("synthetic__security-009", comments=[_comment("z", article="security/made-up-slug")]), + _entry("synthetic__security-001", comments=[_comment("x", "security/secrettext-for-credentials")]), + _entry("synthetic__security-009", comments=[_comment("z", "security/made-up-slug")]), ] report = build_coverage_report(entries, inventory=inventory) assert report.inventory_available is True @@ -106,7 +126,7 @@ def test_with_inventory_flags_zero_and_unknown(self): def test_covered_entry_ids_deduplicated_across_entries(self): article = "security/inherent-permissions-minimal-grant" entries = [ - _entry("synthetic__security-013", comments=[_comment("a", article=article), _comment("b", article=article)]), + _entry("synthetic__security-013", comments=[_comment("a", article), _comment("b", article)]), ] report = build_coverage_report(entries, inventory={article}) assert len(report.covered) == 1