[Security] Mark all procedures in Http Authentication Basic as [NonDebuggable]#7856
Open
[Security] Mark all procedures in Http Authentication Basic as [NonDebuggable]#7856
Conversation
…[NonDebuggable] Mark the ToBase64 local procedure on codeunit "Http Authentication Basic" as [NonDebuggable] so that the SecretText.Unwrap() it performs is not exposed via the debugger if a future refactor stores the unwrapped value in a local variable. Aligns with the same pattern already used by the [NonDebuggable] ToBase64(SecretText) overload in the Base64 Convert module. The AL0796 pragma is no longer needed once the procedure is NonDebuggable, so the disable/restore block is removed. Fixes AB#631826 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
Marks the Basic Auth Base64-encoding helper in the Rest Client’s Http Authentication Basic implementation as non-debuggable to prevent accidental exposure of unwrapped SecretText values during debugging.
Changes:
- Adds
[NonDebuggable]to the localToBase64(SecretText)procedure inHttpAuthenticationBasic.Codeunit.al. - Removes the now-redundant
#pragma warning disable/restore AL0796suppression aroundSecretText.Unwrap().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
AL Documentation Audit
Documentation gaps were detected in the following apps:
- Rest-Client: 98% documentation coverage
To generate documentation, run /al-docs init or /al-docs update using GitHub Copilot CLI or Claude Code.
This review is for awareness to help keep documentation in sync with code changes. It is okay to dismiss this request.
gggdttt
previously approved these changes
Apr 29, 2026
Extends the protection to every procedure in codeunit 2359 instead of just ToBase64. AL does not support marking a codeunit object as [NonDebuggable], so the equivalent is to decorate every method. This hardens the entire credential-handling chain (Initialize overloads and GetAuthorizationHeaders) against future refactors that might introduce debugger-visible intermediates holding the unwrapped password.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Http Authentication Basiccodeunit (Rest Client app, codeunit 2359) as[NonDebuggable]—Initialize(both overloads),IsAuthenticationRequired,GetAuthorizationHeaders, andToBase64. AL does not support[NonDebuggable]at the codeunit object level, so decorating every procedure is the equivalent of "the whole codeunit is non-debuggable."SecretTextpassword throughInitialize-> stores it inGlobalPassword-> composes the basic-auth header inGetAuthorizationHeaders-> base64-encodes it viaToBase64(which callsSecretText.Unwrap()). Marking every entry point[NonDebuggable]is a structural defense-in-depth: it protects the codepath regardless of how the body of any procedure evolves, and it prevents any future refactor that introduces a debugger-visible local from silently exposing the credential.Base64ConvertImpl.Codeunit.al(System Application Base64 Convert), where the equivalentToBase64(SecretString: SecretText): SecretTextoverload is already[NonDebuggable].#pragma warning disable AL0796/restore AL0796block is removed: AL0796 fires onUnwrap()calls in debuggable contexts, so onceToBase64is[NonDebuggable]the warning no longer applies and the suppression becomes dead noise.Scope
[NonDebuggable]. The interface contract (Http Authentication) is unaffected — the attribute is independent of the interface signature, and the sibling implementationHttp Authentication Anonymous(codeunit 2358) handles no secrets and is unchanged.TestBasicAuthenticationtest inHttpAuthenticationTests.Codeunit.alis itself[NonDebuggable]and still exercises the fullGetAuthorizationHeaders->ToBase64chain.Fixes AB#631826