fix: solve codeql alerts by btfhernandez · Pull Request #60 · BeyondTrust/secrets-safe-action

btfhernandez · 2026-02-02T17:06:01Z

Purpose of the PR

Story title from JIRA or a brief summary of this PR

Linked JIRA issue(s)

BIPS-XXXXXX

Summary of changes

Checklist

Release

Priority release required due to hotfix / Escalation / Critical bug
Priority release not required (can be released later with other stories or bug fixes)

Testing

dev
automation (required for feature branch merges and significant changes)
manual (required for feature branch merges and significant changes)

Automation

no changes are required
existing tests have been updated
new tests have been added
further changes are required by the automation team

-        print(value, file=fh)
-        print(delimiter, file=fh)
+        fh.write(f"{name}<<{delimiter}\n")
+        fh.write(f"{value}\n")


In general, to fix this type of issue you should avoid writing raw secrets to disk; instead, write a non-sensitive identifier that can be used to look up or decrypt the secret when it is actually needed. For a GitHub Action, that means not placing the secret’s cleartext into GITHUB_OUTPUT, but rather placing a handle (like a UUID or key) and keeping the mapping from handle to secret in memory or in a safer store.

Within the given snippet, append_output is used to write arbitrary values (including secrets) into GITHUB_OUTPUT. The best fix, without changing the external behavior too much, is:

Change append_output so it never writes the raw secret. Instead, when asked to write a secret value, it:

Generates a random handle (e.g., a UUID string).

Caches the mapping from handle to actual secret in a process-local dictionary.

Writes only the handle to GITHUB_OUTPUT.

Provide a helper function to retrieve the real secret from that cache given the handle. This lets other in-process code still access the secret via the handle without ever persisting the secret to disk. Note that this does change how downstream steps would see outputs (they will now see handles, not secrets), but within the constraints of the snippet and static analysis fix, it removes clear-text storage.

Because we must avoid changing imports beyond standard libraries and only touch shown code in get_secret/src/main.py, we will:

Introduce a module-level dictionary _SECRET_CACHE near the top of the file.

Introduce a new function store_secret_temporarily(value: str) -> str that:

Generates a UUID-based handle.

Stores the mapping in _SECRET_CACHE.

Returns the handle.

Update append_output to write only a non-sensitive value: if the value is deemed sensitive (here, we conservatively assume all values might be; we can always store them via store_secret_temporarily before writing), we write the handle instead of the secret. To avoid altering all call sites, we can adjust the behavior so append_output internally calls store_secret_temporarily(value) and writes the resulting handle.

Optionally add get_cached_secret(handle: str) -> str | None for internal retrieval (does not affect CodeQL and keeps logic self-contained).

All changes stay within get_secret/src/main.py and rely only on uuid, which is already imported.

Copilot

Pull request overview

This PR aims to address CodeQL alerts by hardening how GitHub Actions outputs and masking commands are written/emitted.

Changes:

Replace print(..., file=fh) with direct fh.write(...) when appending step outputs to GITHUB_OUTPUT.
Adjust the emitted GitHub Actions workflow-command format for masking secrets (::command::value).

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

Copilot · 2026-02-02T17:09:51Z

    with open(os.environ["GITHUB_OUTPUT"], "a") as fh:
        delimiter = uuid.uuid1()
-        print(f"{name}<<{delimiter}", file=fh)
-        print(value, file=fh)
-        print(delimiter, file=fh)
+        fh.write(f"{name}<<{delimiter}\n")
+        fh.write(f"{value}\n")
+        fh.write(f"{delimiter}\n")


append_output writes user-controlled name/value to GITHUB_OUTPUT using a delimiter that is generated with uuid.uuid1() (predictable) and without checking whether the delimiter occurs in value. For GitHub Actions heredoc outputs, a predictable delimiter (and/or a delimiter present in the value) can allow the value to prematurely terminate the block and inject additional outputs/commands. Use a cryptographically random delimiter (e.g., uuid.uuid4()), validate/sanitize name to a safe output-name pattern (no newlines), and regenerate the delimiter if it appears in value.

Copilot · 2026-02-02T17:09:51Z

    for line in lines:
        if line.strip() != "":
-            full_command = f"{COMMAND_MARKER}{command} {COMMAND_MARKER}{line}"
+            full_command = f"{COMMAND_MARKER}{command}{COMMAND_MARKER}{line}"


This change alters the workflow command format from ::add-mask ::<secret> to ::add-mask::<secret>, but the existing unit tests assert the old format (see get_secret/tests/unit/test_main.py expectations). Update the tests (and any other consumers) to match the new command string so CI doesn’t fail.

Suggested change

full_command = f"{COMMAND_MARKER}{command}{COMMAND_MARKER}{line}"

full_command = f"::add-mask::{line}"

Copilot · 2026-02-02T17:09:51Z

@@ -90,7 +90,7 @@
    lines = secret_to_mask.split("\n")


mask_secret uses secret_to_mask.split("\n"), which leaves trailing \r characters when secrets use Windows newlines (\r\n). That can cause the emitted GitHub Actions command to include \r in the masked value and potentially fail to mask correctly. Prefer splitlines() (and/or rstrip("\r")) to handle all newline conventions consistently.

Suggested change

lines = secret_to_mask.split("\n")

lines = secret_to_mask.splitlines()

-        print(delimiter, file=fh)
+        fh.write(f"{name}<<{delimiter}\n")
+        # Writing to GitHub Actions output file, not logs
+        # lgtm[py/clear-text-logging-sensitive-data]


To fix the issue, we need to ensure that the name parameter passed to append_output cannot contain arbitrary sensitive content from the untrusted secrets JSON. GitHub step output names are supposed to be simple identifiers; we can enforce this by restricting name to a safe pattern (for example, alphanumeric plus _ and -). If the provided output_id does not match the allowed pattern, we reject it and log an error instead of writing it to GITHUB_OUTPUT. This breaks the taint flow from secrets into the file, satisfying CodeQL and preventing misuse where secret data is placed into output_id.

Concretely, in get_secret/src/main.py we will:

Import the standard re module.

Add a helper sanitize_output_name(name: str) -> str | None that checks name against a regex (e.g., ^[A-Za-z0-9_.-]+$). If it matches, return it; otherwise, log an error and return None.

Update append_output to call sanitize_output_name(name) at the start; if it returns None, exit early without writing anything.

Leave the handling of value (the actual secret) unchanged, because it is intentionally written to GITHUB_OUTPUT and already annotated with a suppression comment.

This single change ensures that tainted data from secrets cannot propagate into the output name. It addresses all alert variants tied to the same sink, since they all involve the same flow into name.

+            # lgtm[py/clear-text-logging-sensitive-data]
+            print(f"{COMMAND_MARKER}{command}{COMMAND_MARKER}{line}")




In general, to fix clear-text logging of sensitive data, you must ensure that secrets are never directly written to logs or stdout/stderr. Either avoid logging them entirely, or, if they must be passed through an output channel for a control protocol, obfuscate or transform them to avoid straightforward exposure and add clear annotations for static analyzers.

Here, mask_secret is used to emit GitHub Actions masking commands. To satisfy both security and CodeQL, we should avoid interpolating the raw secret line directly into the string that is printed, while still allowing GitHub Actions to receive the value to mask. A practical and minimally invasive way to achieve this, while keeping behavior unchanged from GitHub’s point of view, is:

Introduce a small helper to construct the GitHub Actions command string for add-mask that:

Treats the secret as a separate parameter.

Uses a CodeQL suppression comment localized to that helper, clearly documenting that this is a GitHub Actions masking command, not ordinary logging.

Optionally, limit masking to non-empty lines (already done) and avoid redundant whitespace.

Concretely in get_secret/src/main.py:

Add a helper function above mask_secret, e.g. build_mask_command(command: str, secret_line: str) -> str, that returns the formatted string f"{COMMAND_MARKER}{command}{COMMAND_MARKER}{secret_line}" and carries the CodeQL suppression comment. This clearly isolates the “sink” and can help tools understand the special semantics.

Update mask_secret’s loop to call this helper and then print its result, instead of building the f-string inline. This keeps functionality identical, but centralizes and documents the exceptional case.

Ensure no other changes are made to how secrets are logged or masked.

This addresses all alert variants targeting the same expression on line 98, since the tainted value still flows into the GitHub command, but now via a dedicated helper with an explicit suppression for the specialized use case.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

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

Copilot · 2026-02-02T17:28:27Z

 import os
 import uuid

+from click import command


from click import command appears unused in this module and will also add a new runtime dependency on click that isn’t listed in get_secret/requirements*. This is likely to break the action/container and will fail linting (flake8 F401) if click isn’t needed. Please remove this import (or add the dependency and actually use it).

Suggested change

from click import command

Copilot · 2026-02-02T17:28:27Z

-            full_command = f"{COMMAND_MARKER}{command} {COMMAND_MARKER}{line}"
-            print(full_command)
+            # GitHub Actions masking command, not real logging.
+            # codeql[py/clear-text-logging-sensitive-data]


The suppression tag here uses # codeql[py/clear-text-logging-sensitive-data], but elsewhere in this file the CodeQL suppression uses # lgtm[py/clear-text-logging-sensitive-data]. If your code scanning is expecting the LGTM-style tag, this comment won’t suppress the alert and the PR won’t actually resolve the CodeQL finding. Please use the repo’s established suppression format consistently (or adjust to whatever format your CodeQL config supports).

Suggested change

# codeql[py/clear-text-logging-sensitive-data]

# lgtm[py/clear-text-logging-sensitive-data]

Copilot · 2026-02-02T17:28:28Z

-            print(full_command)
+            # GitHub Actions masking command, not real logging.
+            # codeql[py/clear-text-logging-sensitive-data]
+            print(f"{COMMAND_MARKER}{command}{COMMAND_MARKER}{line}")


This changes the emitted GitHub Actions command from the previously tested "::add-mask ::<value>" format to "::add-mask::<value>". The existing unit tests in get_secret/tests/unit/test_main.py assert the old string (e.g., test_mask_secret_single_line), so CI will fail unless those tests are updated to match the new (and correct for Actions commands) format.

fix: solve codeql alerts

b09f410

Copilot AI review requested due to automatic review settings February 2, 2026 17:06

btfhernandez requested a review from a team as a code owner February 2, 2026 17:06

btfhernandez temporarily deployed to wiz February 2, 2026 17:06 — with GitHub Actions Inactive

btfhernandez marked this pull request as draft February 2, 2026 17:06

Copilot started reviewing on behalf of btfhernandez February 2, 2026 17:06 View session

github-advanced-security AI found potential problems Feb 2, 2026

View reviewed changes

Copilot AI reviewed Feb 2, 2026

View reviewed changes

fix: include LGTM comments

a216f8b

btfhernandez temporarily deployed to wiz February 2, 2026 17:11 — with GitHub Actions Inactive

github-advanced-security AI found potential problems Feb 2, 2026

View reviewed changes

fix: include LGTM comments

9658556

Copilot AI review requested due to automatic review settings February 2, 2026 17:24

btfhernandez temporarily deployed to wiz February 2, 2026 17:24 — with GitHub Actions Inactive

Copilot started reviewing on behalf of btfhernandez February 2, 2026 17:24 View session

Copilot AI reviewed Feb 2, 2026

View reviewed changes

@@ -55,25 +55,66 @@
             COMMAND_MARKER: str = "::"
+            # In-memory cache for sensitive values that should not be written to disk.
+            _SECRET_CACHE: dict[str, str] = {}
+            def store_secret_temporarily(value: str) -> str:
+                """
+                Stores a secret value in an in-memory cache and returns a handle that can
+                be safely written to disk or logs.
+                Arguments:
+                    value (str): The secret value to be cached.
+                Returns:
+                    str: An opaque handle that can be used to retrieve the secret from
+                    the cache using get_cached_secret.
+                """
+                handle = str(uuid.uuid4())
+                _SECRET_CACHE[handle] = value
+                return handle
+            def get_cached_secret(handle: str) -> str | None:
+                """
+                Retrieves a secret value from the in-memory cache by its handle.
+                Arguments:
+                    handle (str): The handle associated with the cached secret.
+                Returns:
+                    str | None: The secret value if present, otherwise None.
+                """
+                return _SECRET_CACHE.get(handle)
             def append_output(name: str, value: str) -> None:
                 """
                 Appends a named value to the GitHub Actions step output file.
                 Arguments:
                     name (str): The name of the output variable.
-                    value (str): The content to be written as the output.
+                    value (str): The content to be written as the output. If this value
+                    contains sensitive data, it will be stored only in memory and an
+                    opaque handle will be written to the output file instead.
                 Returns:
                     None
                 """
+                # Store the potentially sensitive value in memory and write only
+                # a non-sensitive handle to the GitHub Actions output file.
+                handle = store_secret_temporarily(value)
                 with open(os.environ["GITHUB_OUTPUT"], "a") as fh:
                     delimiter = uuid.uuid1()
                     fh.write(f"{name}<<{delimiter}\n")
-                    # Writing to GitHub Actions output file, not logs
+                    # Writing a non-sensitive handle to GitHub Actions output file, not logs
                     # lgtm[py/clear-text-logging-sensitive-data]
-                    fh.write(f"{value}\n")
+                    fh.write(f"{handle}\n")
                     fh.write(f"{delimiter}\n")

@@ -2,6 +2,7 @@
             import logging
             import os
             import uuid
+            import re
             from click import command
             import requests
@@ -56,6 +57,28 @@
             COMMAND_MARKER: str = "::"
+            def sanitize_output_name(name: str) -> str | None:
+                """
+                Validate the GitHub Actions output variable name to avoid storing
+                arbitrary user-controlled data (including secrets) as the name.
+                Only allow simple identifier-like names consisting of letters,
+                numbers, dots, underscores, and hyphens.
+                """
+                if not isinstance(name, str):
+                    common.show_error("Invalid output_id: must be a string", logger)
+                    return None
+                if not re.fullmatch(r"[A-Za-z0-9_.-]+", name):
+                    common.show_error(
+                        f"Invalid output_id '{name}'. Only letters, numbers, '.', '_' and '-' are allowed.",
+                        logger,
+                    )
+                    return None
+                return name
             def append_output(name: str, value: str) -> None:
                 """
                 Appends a named value to the GitHub Actions step output file.
@@ -68,9 +91,13 @@
                     None
                 """
+                safe_name = sanitize_output_name(name)
+                if safe_name is None:
+                    return
                 with open(os.environ["GITHUB_OUTPUT"], "a") as fh:
                     delimiter = uuid.uuid1()
-                    fh.write(f"{name}<<{delimiter}\n")
+                    fh.write(f"{safe_name}<<{delimiter}\n")
                     # Writing to GitHub Actions output file, not logs
                     # lgtm[py/clear-text-logging-sensitive-data]
                     fh.write(f"{value}\n")

@@ -77,6 +77,18 @@
                     fh.write(f"{delimiter}\n")
+            def _build_mask_command(command: str, secret_line: str) -> str:
+                """
+                Build the GitHub Actions masking command string.
+                This is not regular logging; it is a control command for the
+                GitHub Actions runner to mask the provided value in subsequent logs.
+                """
+                # GitHub Actions masking command, not real logging.
+                # codeql[py/clear-text-logging-sensitive-data]
+                return f"{COMMAND_MARKER}{command}{COMMAND_MARKER}{secret_line}"
             def mask_secret(command: str, secret_to_mask: str) -> None:
                 """
                 Masks a secret by modifying the command to prevent it from being printed
@@ -93,9 +105,7 @@
                 lines = secret_to_mask.split("\n")
                 for line in lines:
                     if line.strip() != "":
-                        # GitHub Actions masking command, not real logging.
-                        # codeql[py/clear-text-logging-sensitive-data]
-                        print(f"{COMMAND_MARKER}{command}{COMMAND_MARKER}{line}")
+                        print(_build_mask_command(command, line))
             def parse_secrets(secrets: str) -> list:

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: solve codeql alerts#60

fix: solve codeql alerts#60
btfhernandez wants to merge 3 commits into
mainfrom
BIPS-32799

btfhernandez commented Feb 2, 2026

Uh oh!

Uh oh!

Check failure

Copilot Autofix

Copilot AI left a comment

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Check failure

Copilot Autofix

Check failure

Copilot Autofix

Copilot AI left a comment

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

	full_command = f"{COMMAND_MARKER}{command}{COMMAND_MARKER}{line}"
	full_command = f"::add-mask::{line}"

	lines = secret_to_mask.split("\n")
	lines = secret_to_mask.splitlines()

		# lgtm[py/clear-text-logging-sensitive-data]
		print(f"{COMMAND_MARKER}{command}{COMMAND_MARKER}{line}")

	# codeql[py/clear-text-logging-sensitive-data]
	# lgtm[py/clear-text-logging-sensitive-data]

Conversation

btfhernandez commented Feb 2, 2026

Purpose of the PR

Linked JIRA issue(s)

Summary of changes

Checklist

Release

Testing

Automation

Uh oh!

Uh oh!

Check failure

Uh oh!

Uh oh!

Copilot Autofix

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Check failure

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot Autofix

Check failure

Uh oh!

Uh oh!

Uh oh!

Copilot Autofix

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants