Skip to content

[Security] Mark all procedures in Http Authentication Basic as [NonDebuggable]#7856

Open
onbuyuka wants to merge 2 commits intomainfrom
bugs/631826-http-auth-basic-nondebuggable
Open

[Security] Mark all procedures in Http Authentication Basic as [NonDebuggable]#7856
onbuyuka wants to merge 2 commits intomainfrom
bugs/631826-http-auth-basic-nondebuggable

Conversation

@onbuyuka
Copy link
Copy Markdown
Contributor

@onbuyuka onbuyuka commented Apr 27, 2026

Summary

  • Mark every procedure in the Http Authentication Basic codeunit (Rest Client app, codeunit 2359) as [NonDebuggable]Initialize (both overloads), IsAuthenticationRequired, GetAuthorizationHeaders, and ToBase64. AL does not support [NonDebuggable] at the codeunit object level, so decorating every procedure is the equivalent of "the whole codeunit is non-debuggable."
  • The credential-handling chain in this codeunit takes a SecretText password through Initialize -> stores it in GlobalPassword -> composes the basic-auth header in GetAuthorizationHeaders -> base64-encodes it via ToBase64 (which calls SecretText.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.
  • Aligns with the existing convention in Base64ConvertImpl.Codeunit.al (System Application Base64 Convert), where the equivalent ToBase64(SecretString: SecretText): SecretText overload is already [NonDebuggable].
  • The #pragma warning disable AL0796 / restore AL0796 block is removed: AL0796 fires on Unwrap() calls in debuggable contexts, so once ToBase64 is [NonDebuggable] the warning no longer applies and the suppression becomes dead noise.

Scope

  • All procedures in codeunit 2359 are decorated with [NonDebuggable]. The interface contract (Http Authentication) is unaffected — the attribute is independent of the interface signature, and the sibling implementation Http Authentication Anonymous (codeunit 2358) handles no secrets and is unchanged.
  • Behavior is unchanged. The existing TestBasicAuthentication test in HttpAuthenticationTests.Codeunit.al is itself [NonDebuggable] and still exercises the full GetAuthorizationHeaders -> ToBase64 chain.

Fixes AB#631826

…[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]>
Copilot AI review requested due to automatic review settings April 27, 2026 10:50
@onbuyuka onbuyuka requested a review from a team as a code owner April 27, 2026 10:50
@onbuyuka onbuyuka changed the title Bug 631826: [Security] Mark ToBase64 in Http Authentication Basic as [NonDebuggable] [Security] Mark ToBase64 in Http Authentication Basic as [NonDebuggable] Apr 27, 2026
@onbuyuka onbuyuka enabled auto-merge (squash) April 27, 2026 10:51
@github-actions github-actions Bot added this to the Version 29.0 milestone Apr 27, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 local ToBase64(SecretText) procedure in HttpAuthenticationBasic.Codeunit.al.
  • Removes the now-redundant #pragma warning disable/restore AL0796 suppression around SecretText.Unwrap().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
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.
@onbuyuka onbuyuka changed the title [Security] Mark ToBase64 in Http Authentication Basic as [NonDebuggable] [Security] Mark all procedures in Http Authentication Basic as [NonDebuggable] Apr 29, 2026
Copy link
Copy Markdown
Member

@WaelAbuSeada WaelAbuSeada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants