Prevent ACR refresh token exposure in EXECVE audit logs by ehvs · Pull Request #4675 · Azure/ARO-RP

ehvs · 2026-03-10T17:04:06Z

Which issue this PR addresses:

Replace az acr login --name with az acr login --expose-token + podman login --password-stdin in pull_container_images() to prevent the ACR refresh token from appearing as a command-line argument in auditd EXECVE records.

https://issues.redhat.com/browse/ARO-24815

why we need it:

During RP/Gateway VMSS bootstrap, pull_container_images() calls az acr login --name $registry. The Azure CLI internally spawns a docker login --password $token subprocess, which exposes the ACR refresh token as a plaintext command-line argument. This is captured by auditd EXECVE syscall logging.

podman --password-stdin should be used rather than az acr log to prevent the secret variable from being shown in shell output.

What is PR does

Tip

Files that directly fix ARO-24815 are (to assist reviewing changes):

pkg/deploy/generator/scripts/rpVMSS.sh
pkg/deploy/generator/scripts/util-common.sh

Unsets xtrace shell option while working with sensitive information. Variables containing sensitive information are written to output when xtrace is set.
- While logging into Azure acr via podman
- While updating system TLS certificates
Addresses shellcheck warnings
- The warnings that can be safely disregarded have been disabled
- Some warnings were valid, that code has been refactored

General improvements to improve readability

All function and variable descriptions have been refactored to have a uniform style.
Command/function calls with more than three options have been broken up into multiple lines.

Test plan for issue:

Deploying to INT to

Ensure the problem is resolved
Ensure no new errors/failures were introduced

Successfully deployed in pipelines:

INT
Canary
- The script succeeds until the aro image pull is attempted. It tries to pull an image based on the tag, which apparently is no longer created.
Re-releasing to INT to confirm no regression after rebasing from master
- Pipeline run

Copilot

Pull request overview

This PR updates the VMSS bootstrapping image-pull flow to avoid exposing ACR credentials in process arguments by switching from az acr login to token-based login and podman login --password-stdin.

Changes:

Replace az acr login --name <registry> with az acr login --expose-token + podman login --password-stdin in pull_container_images().
Derive ACR registry hostname from ACRRESOURCEID for use with podman login.

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

pkg/deploy/generator/scripts/util-system.sh

Copilot

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

Copilot

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

swiencki · 2026-03-18T18:08:09Z

The new xtrace helpers can abort bootstrap under set -e. xtrace_unset() returns non-zero when tracing is already off, and xtrace_set() returns non-zero when tracing was previously on. Because both VMSS bootstrap scripts run with errexit, the new wrapper around the ACR login path can terminate the script instead of just suppressing tracing.

ARO-RP/pkg/deploy/generator/scripts/util-common.sh

Lines 54 to 71 in 467e224

    
           #   * Sets XTRACE_IS_SET=XTRACE_SET (if true) or XTRACE_IS_SET=XTRACE_UNSET (if false). 
        
           xtrace_set_capture() { 
        
               [[ $- =~ "x" ]] && XTRACE_IS_SET=$XTRACE_SET || XTRACE_IS_SET=$XTRACE_UNSET 
        
           } 
        
           # xtrace_unset() 
        
           # 
        
           # Un-sets xtrace (if set) 
        
           xtrace_unset() { 
        
               (( XTRACE_IS_SET == XTRACE_SET )) && set +x 
        
           } 
        
           # xtrace_set() 
        
           # 
        
           # Sets xtrace (if unset) 
        
           xtrace_set() { 
        
               (( XTRACE_IS_SET == XTRACE_UNSET )) && set -x 
        
           }

hlipsig · 2026-03-18T18:18:37Z

@copilot please review

Copilot · 2026-03-18T18:18:48Z

@hlipsig I've opened a new pull request, #4692, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

s-fairchild · 2026-03-18T18:24:42Z

The new xtrace helpers can abort bootstrap under set -e. xtrace_unset() returns non-zero when tracing is already off, and xtrace_set() returns non-zero when tracing was previously on. Because both VMSS bootstrap scripts run with errexit, the new wrapper around the ACR login path can terminate the script instead of just suppressing tracing.

ARO-RP/pkg/deploy/generator/scripts/util-common.sh

Lines 54 to 71 in 467e224

# * Sets XTRACE_IS_SET=XTRACE_SET (if true) or XTRACE_IS_SET=XTRACE_UNSET (if false).

xtrace_set_capture() {

[[ $- =~ "x" ]] && XTRACE_IS_SET=$XTRACE_SET || XTRACE_IS_SET=$XTRACE_UNSET

}

# xtrace_unset()

#

# Un-sets xtrace (if set)

xtrace_unset() {

(( XTRACE_IS_SET == XTRACE_SET )) && set +x

}

# xtrace_set()

#

# Sets xtrace (if unset)

xtrace_set() {

(( XTRACE_IS_SET == XTRACE_UNSET )) && set -x

}

@swiencki This is true. I've tested locally with a short script sourcing util-common.sh and running xtrace_set and xtrace_unset. Not super relevant, but running xtrace_set in succession does not cause failure (return non zero). However running xtrace_set in succession does cause failure (return non zero). Not sure why.

Anyways I've pushed an update that returns 0 if the conditional test for XTRACE_IS_SET is false.

s-fairchild · 2026-03-18T18:29:49Z

@copilot please review

pkg/deploy/generator/scripts/util-common.sh

github-actions · 2026-03-18T21:43:10Z

Please rebase pull request.

Copilot

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

github-actions · 2026-03-25T00:07:04Z

Please rebase pull request.

…e acr login secret Reference: https://portal.microsofticm.com/imp/v5/incidents/details/752908291/summary Add unset xtrace while configuring tls certificates * To be extra safe and ensure no certificate details are logged via shell tracing. Add shell check comments, Update function descriptions * Add shellcheck comments for warnings that can be safely ignored. * Improve function readability by formatting them all in the same style. Address shellcheck warnings in bootstrap scripts * Several warnings were unaddressed. Some of them were disabled as they can be safely ignored. * Others were refactored as the warnings were legitimate.

cadenmarchese · 2026-03-25T20:09:14Z

/azp run ci

azure-pipelines · 2026-03-25T20:09:25Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot AI review requested due to automatic review settings March 10, 2026 17:04

ehvs requested review from alcasim, bennerv, cadenmarchese, hawkowl, hlipsig, jharrington22, kimorris27, mociarain, mrWinston, rogbas, sankur-codes, tiguelu, tsatam, tuxerrante, ventifus, wanghaoran1988 and yjst2012 as code owners March 10, 2026 17:04

Copilot started reviewing on behalf of ehvs March 10, 2026 17:04 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

pkg/deploy/generator/scripts/util-system.sh Outdated Show resolved Hide resolved

pkg/deploy/generator/scripts/util-system.sh Outdated Show resolved Hide resolved

pkg/deploy/generator/scripts/util-system.sh Outdated Show resolved Hide resolved

s-fairchild reviewed Mar 10, 2026

View reviewed changes

pkg/deploy/generator/scripts/util-system.sh Outdated Show resolved Hide resolved

s-fairchild reviewed Mar 10, 2026

View reviewed changes

pkg/deploy/generator/scripts/util-system.sh Outdated Show resolved Hide resolved

s-fairchild force-pushed the ehvs/ARO-24815 branch 2 times, most recently from fff96c9 to bd9f985 Compare March 17, 2026 18:04

Copilot AI review requested due to automatic review settings March 17, 2026 18:04

Copilot AI reviewed Mar 17, 2026

View reviewed changes

s-fairchild force-pushed the ehvs/ARO-24815 branch from bd9f985 to 0c59144 Compare March 17, 2026 18:32

Copilot AI review requested due to automatic review settings March 17, 2026 19:16

Copilot AI reviewed Mar 17, 2026

View reviewed changes

s-fairchild force-pushed the ehvs/ARO-24815 branch from a068f9c to 467e224 Compare March 17, 2026 19:31

s-fairchild added ready-for-review loki labels Mar 18, 2026

Copilot AI mentioned this pull request Mar 18, 2026

Fix xtrace helper logic inversion and set -e abort risk in util-common.sh #4692

Closed

s-fairchild force-pushed the ehvs/ARO-24815 branch from 467e224 to 865dc1d Compare March 18, 2026 18:23

Copilot AI review requested due to automatic review settings March 18, 2026 18:23

Copilot AI reviewed Mar 18, 2026

View reviewed changes

rajdeepc2792 reviewed Mar 18, 2026

View reviewed changes

pkg/deploy/generator/scripts/util-common.sh Show resolved Hide resolved

s-fairchild force-pushed the ehvs/ARO-24815 branch from 865dc1d to 3fd4684 Compare March 18, 2026 21:42

github-actions bot added needs-rebase branch needs a rebase and removed ready-for-review labels Mar 18, 2026

s-fairchild force-pushed the ehvs/ARO-24815 branch from 3fd4684 to d002497 Compare March 19, 2026 13:49

Copilot AI review requested due to automatic review settings March 19, 2026 13:49

Copilot AI reviewed Mar 19, 2026

View reviewed changes

github-actions bot removed the needs-rebase branch needs a rebase label Mar 19, 2026

tsatam approved these changes Mar 20, 2026

View reviewed changes

github-actions bot added the needs-rebase branch needs a rebase label Mar 25, 2026

s-fairchild added the next-release To be included in the next RP release rollout label Mar 25, 2026

s-fairchild force-pushed the ehvs/ARO-24815 branch from d002497 to 461999c Compare March 25, 2026 14:31

github-actions bot removed the needs-rebase branch needs a rebase label Mar 25, 2026

tsatam approved these changes Mar 25, 2026

View reviewed changes

Conversation

ehvs commented Mar 10, 2026 • edited by s-fairchild Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue this PR addresses:

why we need it:

What is PR does

Test plan for issue:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

swiencki commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hlipsig commented Mar 18, 2026

Uh oh!

Copilot AI commented Mar 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

s-fairchild commented Mar 18, 2026

Uh oh!

s-fairchild commented Mar 18, 2026

Uh oh!

Uh oh!

github-actions bot commented Mar 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 25, 2026

Uh oh!

cadenmarchese commented Mar 25, 2026

Uh oh!

azure-pipelines bot commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

ehvs commented Mar 10, 2026 •

edited by s-fairchild

Loading

swiencki commented Mar 18, 2026 •

edited

Loading