Skip to content

Improve token handling and cleanup in docker.md #14365

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ianwallen wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Improve token handling and cleanup in docker.md #14365

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 53 additions & 25 deletions docs/pipelines/agents/docker.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,42 +316,58 @@ Next, create the Dockerfile.
#!/bin/bash
set -e

if [ -z "${AZP_URL}" ]; then
echo 1>&2 "error: missing AZP_URL environment variable"
exit 1
fi
# Load a token either from the environment variable or by using the service principal credentials.
load_azp_token() {
if [ -n "$AZP_CLIENTID" ]; then
Comment thread
ianwallen marked this conversation as resolved.
if [ -z "$AZP_CLIENTSECRET" ]; then
echo 1>&2 "error: AZP_CLIENTSECRET must be set when AZP_CLIENTID is used"
exit 1
fi

if [ -z "$AZP_TENANTID" ]; then
echo 1>&2 "error: AZP_TENANTID must be set when AZP_CLIENTID is used"
exit 1
fi

echo "Using service principal credentials to get token"
az login --allow-no-subscriptions --service-principal --username "$AZP_CLIENTID" --password "$AZP_CLIENTSECRET" --tenant "$AZP_TENANTID"
# adapted from https://learn.microsoft.com/en-us/azure/databricks/dev-tools/user-aad-token
AZP_TOKEN=$(az account get-access-token --query accessToken --output tsv)
echo "Token retrieved"

# Ensure credentials are not visible to the agent by un-exporting the variable
export -n AZP_CLIENTSECRET
Comment on lines +338 to +339
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

export -n only removes the variables from the exported environment; it does not clear the values from the current shell. Since this script can run for a long time, AZP_CLIENTSECRET (and potentially AZP_TOKEN) remain accessible within the process. Consider explicitly unset-ing these variables after they are no longer needed (e.g., after successfully writing the token to AZP_TOKEN_FILE) to reduce secret exposure. (Optional hardening: ensure any Azure CLI cached credentials are stored in a disposable location and removed on exit.)

Copilot uses AI. Check for mistakes.
fi

if [ -n "$AZP_CLIENTID" ]; then
echo "Using service principal credentials to get token"
az login --allow-no-subscriptions --service-principal --username "$AZP_CLIENTID" --password "$AZP_CLIENTSECRET" --tenant "$AZP_TENANTID"
# adapted from https://learn.microsoft.com/en-us/azure/databricks/dev-tools/user-aad-token
AZP_TOKEN=$(az account get-access-token --query accessToken --output tsv)
echo "Token retrieved"
fi
if [ -z "${AZP_TOKEN_FILE}" ]; then
if [ -z "${AZP_TOKEN}" ]; then
echo 1>&2 "error: missing AZP_TOKEN environment variable"
exit 1
fi

if [ -z "${AZP_TOKEN_FILE}" ]; then
if [ -z "${AZP_TOKEN}" ]; then
echo 1>&2 "error: missing AZP_TOKEN environment variable"
exit 1
AZP_TOKEN_FILE="$(dirname "$0")/.token"
fi

AZP_TOKEN_FILE="/azp/.token"
echo -n "${AZP_TOKEN}" > "${AZP_TOKEN_FILE}"
fi

unset AZP_CLIENTSECRET
unset AZP_TOKEN
if [ -n "${AZP_TOKEN}" ]; then
echo -n "${AZP_TOKEN}" > "${AZP_TOKEN_FILE}"
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

When AZP_TOKEN_FILE is provided (non-empty) and neither AZP_TOKEN nor service principal creds are used, load_azp_token doesn’t validate that the token file actually exists and is readable/non-empty. That can lead to a later, less actionable failure when configuring/removing the agent. Consider adding a fast-fail validation for the AZP_TOKEN_FILE path (existence + readability and/or non-empty) when AZP_TOKEN isn’t being written.

Suggested change
echo -n "${AZP_TOKEN}" > "${AZP_TOKEN_FILE}"
echo -n "${AZP_TOKEN}" > "${AZP_TOKEN_FILE}"
else
if [ ! -f "${AZP_TOKEN_FILE}" ]; then
echo 1>&2 "error: AZP_TOKEN_FILE does not exist: ${AZP_TOKEN_FILE}"
exit 1
fi
if [ ! -r "${AZP_TOKEN_FILE}" ]; then
echo 1>&2 "error: AZP_TOKEN_FILE is not readable: ${AZP_TOKEN_FILE}"
exit 1
fi
if [ ! -s "${AZP_TOKEN_FILE}" ]; then
echo 1>&2 "error: AZP_TOKEN_FILE is empty: ${AZP_TOKEN_FILE}"
exit 1
fi

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

The indentation on the echo line is inconsistent with the surrounding block, which makes the embedded script harder to read and maintain in the docs. Align the indentation to match the rest of the function.

Suggested change
echo -n "${AZP_TOKEN}" > "${AZP_TOKEN_FILE}"
echo -n "${AZP_TOKEN}" > "${AZP_TOKEN_FILE}"

Copilot uses AI. Check for mistakes.
fi

if [ -n "${AZP_WORK}" ]; then
mkdir -p "${AZP_WORK}"
fi
# Ensure credentials are not visible to the agent by un-exporting the variable
export -n AZP_TOKEN
Comment on lines +355 to +356
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

export -n only removes the variables from the exported environment; it does not clear the values from the current shell. Since this script can run for a long time, AZP_CLIENTSECRET (and potentially AZP_TOKEN) remain accessible within the process. Consider explicitly unset-ing these variables after they are no longer needed (e.g., after successfully writing the token to AZP_TOKEN_FILE) to reduce secret exposure. (Optional hardening: ensure any Azure CLI cached credentials are stored in a disposable location and removed on exit.)

Suggested change
# Ensure credentials are not visible to the agent by un-exporting the variable
export -n AZP_TOKEN
# Ensure credentials are not retained in the current shell after writing the token file
unset AZP_TOKEN

Copilot uses AI. Check for mistakes.
}

# Cleanup function to remove the agent configuration.
cleanup() {
trap "" EXIT

if [ -e ./config.sh ]; then
print_header "Cleanup. Removing Azure Pipelines agent..."

# Ensure we have a new token if using service principal credentials, as the old one might have expired between the time it was waiting to run a job and now.
if [ -n "$AZP_CLIENTID" ]; then
load_azp_token
fi

# If the agent has some running jobs, the configuration removal process will fail.
# So, give it some time to finish the job.
while true; do
Expand All @@ -369,8 +385,20 @@ Next, create the Dockerfile.
echo -e "\n${lightcyan}$1${nocolor}\n"
}

if [ -z "${AZP_URL}" ]; then
echo 1>&2 "error: missing AZP_URL environment variable"
exit 1
fi

# Load the AZP token for initial setup.
load_azp_token

if [ -n "${AZP_WORK}" ]; then
mkdir -p "${AZP_WORK}"
fi

# Let the agent ignore the token env variables
export VSO_AGENT_IGNORE="AZP_TOKEN,AZP_TOKEN_FILE"
export VSO_AGENT_IGNORE="AZP_TOKEN,AZP_TOKEN_FILE,AZP_CLIENTSECRET"

Comment thread
ianwallen marked this conversation as resolved.
print_header "1. Determining matching Azure Pipelines agent..."

Expand Down
Loading