fix(security): restrict node secret file perms and avoid logging AD realm password by calvin197 · Pull Request #9776 · Azure/AgentBaker

Calvin S. (calvin197) · 2026-10-14T04:05:21Z

What this PR does / why we need it:
Hardens three node-provisioning scripts so secrets aren't exposed via file permissions or provisioning logs. Addresses Mythos security findings (tracked under ADO 37518878):

• bootstrap-kubeconfig (ADO 37518906/911/915) — /var/lib/kubelet/bootstrap-kubeconfig embeds the TLS bootstrap token but was written world-readable ( chmod 0644 ). Tightened to chmod 0600 so unprivileged local users can't read the token.
• kubelet client key (ADO 37518920/916) — /etc/kubernetes/certs/client.key (kubelet client private key) was written with no explicit mode (umask-dependent, typically 0644). Now touch + chmod 0600 before writing, so there's never a world-readable window.
• AD realm password in logs (ADO 37518908) — setup-custom-search-domains.sh runs under set -x , which echoed the AD realm-join password into provisioning logs. Wrapped the realm join in set +x / set -x .

No functional behavior change — only file modes and log-tracing scope. Validated with go test ./pkg/agent/... and aks-node-controller ./parser/... (both pass), bash -n on all three scripts, and confirmed no ShellSpec assertions reference the changed lines.

Which issue(s) this PR fixes:

Fixes #

…ealm password Addresses Mythos findings tracked under ADO 37518878: - 37518906/911/915: bootstrap-kubeconfig embeds the TLS bootstrap token but was written world-readable (chmod 0644). Restrict to 0600 so unprivileged local users can't read the token. - 37518920/916: the kubelet client private key (/etc/kubernetes/certs/client.key) was written with no explicit mode (umask-dependent, typically 0644). touch + chmod 0600 before writing so it's never world-readable. - 37518908: setup-custom-search-domains.sh runs under `set -x`, echoing the AD realm join password into provisioning logs. Wrap the realm join in `set +x` / `set -x`. Verified: go test ./pkg/agent/... and aks-node-controller ./parser/... pass; bash -n on all three scripts; no ShellSpec assertions reference the changed lines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 350c2577-7031-4d0d-adc1-e99d053fc981

github-actions · 2026-10-14T04:08:35Z

Windows Unit Test Results

3 files 18 suites 49s ⏱️
563 tests 563 ✅ 0 💤 0 ❌
585 runs 585 ✅ 0 💤 0 ❌

Results for commit 9656b6f.

♻️ This comment has been updated with latest results.

Copilot

🟡 Changes recommended

Restoring tracing masks realm-join failures, and the new security invariants lack required ShellSpec coverage.

3 open findings

What changed in this PR

Hardens Linux node provisioning against secret exposure.

Changes:

Restricts kubelet private key and bootstrap kubeconfig permissions.
Suppresses tracing while passing the AD realm password.

File	Description
`setup-custom-search-domains.sh`	Disables tracing around realm join.
`cse_config.sh`	Sets client key permissions to `0600`.
`cse_config_kubelet.sh`	Sets bootstrap kubeconfig permissions to `0600`.

🧠 Review effort: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…-secret-hardening # Conflicts: # parts/linux/cloud-init/artifacts/cse_config_kubelet.sh Co-authored-by: calvin197 <44663669+calvin197@users.noreply.github.com>

Co-authored-by: calvin197 <44663669+calvin197@users.noreply.github.com>

Copilot

🟡 Changes recommended

Restoring command tracing masks failed domain joins and bypasses provisioning error handling.

3 open issues

1 resolved since last review

Preserve realm join failure status before restoring shell tracing

_{Review effort: Balanced}

Co-authored-by: calvin197 <44663669+calvin197@users.noreply.github.com>

Copilot

🟡 Changes recommended

The new client-key test can permanently delete the host’s kubelet credential when run natively as root.

3 open issues

1 resolved since last review

Preserve failed realm join exit status

_{Review effort: Balanced}

This reverts commit 32b2d80.

Copilot

🟡 Changes recommended

Client-key creation still leaves a brief world-readable window that can allow retained access to the private key.

4 open issues

_{Review effort: Balanced}

…om touching real host path Addresses two issues flagged by Copilot's PR review on #9776: - cse_config.sh: touch+chmod left a brief window where client.key was world-readable (default umask 022) before chmod tightened it to 0600. A process with an already-open fd from that window retains read access even after chmod. Create the file under a scoped umask 077 so it is never world-readable, and keep the chmod to also cover pre-existing files. Path is now overridable via $CLIENT_KEY_PATH for testing. - cse_config_spec.sh: the new client-key permissions test deleted and overwrote the real /etc/kubernetes/certs/client.key path. Per spec/README.md, ShellSpec can be run natively as root instead of in the recommended container, which would destroy a real node's kubelet credential with no restoration. Test now uses a throwaway mktemp dir. Verified locally: bash -n on both files, and the full cse_config_spec.sh ShellSpec suite (modern bash 5.3 + GNU stat, matching CI) - client-key test passes, confirmed /etc/kubernetes/certs/client.key is never touched by the test run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 350c2577-7031-4d0d-adc1-e99d053fc981

Copilot

🟢 Approval recommended

Static review found no blocking issues in the scoped hardening changes and regression tests; tests were not executed during this review.

3 open issues

1 resolved since last review

Create new key files with restrictive permissions

_{Review effort: Balanced}

…chmod failures; stop test from touching real host path Addresses issues flagged by Copilot's PR review on #9776: - cse_config.sh: touch+chmod left a brief window where client.key was world-readable (default umask 022) before chmod tightened it to 0600. Create the file under a scoped umask 077 so it's never world-readable at creation time (no TOCTOU window), and keep chmod to also cover pre-existing files. - Guard both touch and chmod explicitly with '|| return 1': this repo's CSE provisioning scripts do NOT run under 'set -e' (verified: neither cse_main.sh nor cse_config.sh enables errexit at the top level; error handling here is manual, via '<cmd> || exit $ERR_X' throughout this file). Without the explicit guard, a chmod failure on a pre-existing, loosely-permissioned client.key would be silently ignored and the private key would be written into that file with its stale, possibly world-readable permissions. Empirically reproduced this exact failure mode locally before the fix, and confirmed it's closed after. - cse_config_spec.sh: the client-key permissions test deleted and overwrote the real /etc/kubernetes/certs/client.key path. Per spec/README.md, ShellSpec can be run natively as root instead of in the recommended container, which would destroy a real node's kubelet credential with no restoration. Test now uses a throwaway mktemp dir. Added coverage for creation-time mode (umask effect, independent of chmod) and for both touch- and chmod-failure paths. Path is now overridable via $CLIENT_KEY_PATH for testing. Verified locally: bash -n on both files, and the full cse_config_spec.sh ShellSpec suite (modern bash 5.3 + GNU stat, matching CI) - all 4 client-key tests pass; only 2 pre-existing, unrelated configureAzureJson failures remain (confirmed present on this branch before this commit, in a different function untouched by this change). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 350c2577-7031-4d0d-adc1-e99d053fc981

Copilot

🟢 Approval recommended

The supplied changes address the prior feedback with focused regression coverage and no identified blocking issues.

3 open issues

_{Review effort: Balanced}

github-actions · 2026-10-22T06:34:13Z

Package	Line Rate	Health
github.com/Azure/agentbaker/aks-node-controller	72%	➖
github.com/Azure/agentbaker/aks-node-controller/common	100%	✔
github.com/Azure/agentbaker/aks-node-controller/helpers	68%	➖
github.com/Azure/agentbaker/aks-node-controller/parser	88%	✔
github.com/Azure/agentbaker/aks-node-controller/pkg/gpu	100%	✔
github.com/Azure/agentbaker/aks-node-controller/pkg/nodeconfigutils	89%	✔
github.com/Azure/agentbaker/aks-node-controller/utils	0%	❌
github.com/Azure/agentbaker/apiserver	24%	❌
github.com/Azure/agentbaker/cmd	0%	❌
github.com/Azure/agentbaker/cmd/starter	0%	❌
github.com/Azure/agentbaker/fuzz/api	0%	❌
github.com/Azure/agentbaker/hotfix/render-nodecustomdata	0%	❌
github.com/Azure/agentbaker/pkg/agent	78%	➖
github.com/Azure/agentbaker/pkg/agent/datamodel	77%	➖
github.com/Azure/agentbaker/pkg/agent/toggles	0%	❌
github.com/Azure/agentbaker/pkg/vhdbuilder/datamodel	87%	✔
Summary	75% (5873 / 7861)	➖

Cameron Meissner (cameronmeissner) · 2026-10-22T23:03:18Z

+            # Guard both operations: if either fails, stop before writing the private key
+            # rather than silently falling through and writing it with whatever (possibly
+            # world-readable) permissions the file ended up with.
+            (umask 077 && touch "${client_key_path}") || return 1


we should use well-defined exit codes here, may need to introduce something new

Replace the generic 'return 1' used when touch/chmod fail while creating the kubelet client private key with a dedicated, well-defined error code (ERR_KUBELET_CLIENT_KEY_PERM_FAIL=247), matching this codebase's existing convention of a unique ERR_* constant per failure mode (defined in cse_helpers.sh) instead of opaque generic statuses. Also fix the call site in cse_main.sh: 'logs_to_events ... configureK8s' was not checked at all (no '|| exit $ERR_X'), meaning a failure here - including this one - was silently swallowed and provisioning continued as if nothing had gone wrong. Added the missing '|| exit' guard so the failure now actually surfaces and aborts provisioning, consistent with every other logs_to_events call site that can fail. configureK8s has no other failure path besides this one, so attributing this specific error code to the whole function's exit is accurate. Updated spec/parts/linux/cloud-init/artifacts/cse_config_spec.sh to assert the specific exit code (not just generic 'failure') on both the touch-failure and chmod-failure paths. Verified: bash -n passes on cse_helpers.sh/cse_config.sh/cse_main.sh; the configureK8s client key permissions suite (4 examples) and the full cse_main_spec.sh suite (17 examples) pass, 0 failures. Two unrelated pre-existing failures in configureAzureJson tests (present before this change too, reproduced on unmodified tree) are untouched by this commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 350c2577-7031-4d0d-adc1-e99d053fc981

github-actions · 2026-10-22T23:47:39Z

Package	Line Rate	Health
test	0%	❌
windows	27%	❌
staging_cse_windows Package 1	65%	➖
debug	0%	❌
provisioningscripts	2%	❌
parts_windows Package 1	73%	➖
Summary	38% (1652 / 6834)	❌

Copilot

🟢 Approval recommended

The focused changes address the prior concerns with guarded key creation, isolated regression coverage, and preserved realm-join failure handling.

No open issues 🎉

3 resolved since last review

_{Review effort: Balanced}

github-actions · 2026-10-22T23:50:00Z

Package	Line Rate	Health
shellspec spec	29%	❌
Summary	29% (4605 / 16090)	❌

Copilot AI balanced review requested due to automatic review settings October 14, 2026 04:05

Calvin S. (calvin197) requested review from a team as code owners October 14, 2026 04:05

Copilot started reviewing on behalf of Calvin S. (calvin197) October 14, 2026 04:06

View session

Calvin S. (calvin197) changed the title ~~fix(security): restrict node secret file perms and avoid logging AD r…~~ fix(security): restrict node secret file perms and avoid logging AD realm password Oct 14, 2026

Copilot AI reviewed Oct 14, 2026

View reviewed changes

Comment thread parts/linux/cloud-init/artifacts/setup-custom-search-domains.sh

Comment thread parts/linux/cloud-init/artifacts/cse_config.sh Outdated

Comment thread parts/linux/cloud-init/artifacts/cse_config_kubelet.sh Outdated

Cameron Meissner (cameronmeissner) reviewed Oct 20, 2026

View reviewed changes

Comment thread parts/linux/cloud-init/artifacts/cse_config_kubelet.sh

Copilot started work on behalf of Calvin S. (calvin197) October 22, 2026 03:20

View session

Merge remote-tracking branch 'origin/main' into security/node-runtime…

fa230aa

…-secret-hardening # Conflicts: # parts/linux/cloud-init/artifacts/cse_config_kubelet.sh Co-authored-by: calvin197 <44663669+calvin197@users.noreply.github.com>

Copilot AI balanced review requested due to automatic review settings October 22, 2026 03:24

Copilot started reviewing on behalf of Calvin S. (calvin197) October 22, 2026 03:25

View session

Preserve realm join failure status

06bf46a

Co-authored-by: calvin197 <44663669+calvin197@users.noreply.github.com>

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Comment thread parts/linux/cloud-init/artifacts/setup-custom-search-domains.sh

Copilot AI balanced review requested due to automatic review settings October 22, 2026 03:29

Copilot started reviewing on behalf of Calvin S. (calvin197) October 22, 2026 03:30

View session

Complete secret-hardening follow-up

32b2d80

Co-authored-by: calvin197 <44663669+calvin197@users.noreply.github.com>

Copilot finished work on behalf of Calvin S. (calvin197) October 22, 2026 03:31

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Comment thread spec/parts/linux/cloud-init/artifacts/cse_config_spec.sh Outdated

Revert "Complete secret-hardening follow-up"

fce9f43

This reverts commit 32b2d80.

Copilot AI balanced review requested due to automatic review settings October 22, 2026 04:19

Copilot started reviewing on behalf of Calvin S. (calvin197) October 22, 2026 04:20

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Comment thread parts/linux/cloud-init/artifacts/cse_config.sh Outdated

Copilot AI balanced review requested due to automatic review settings October 22, 2026 05:51

Copilot started reviewing on behalf of Calvin S. (calvin197) October 22, 2026 05:52

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Copilot started work on behalf of Calvin S. (calvin197) October 22, 2026 05:57

View session

Copilot finished work on behalf of Calvin S. (calvin197) October 22, 2026 06:00

Copilot started work on behalf of Calvin S. (calvin197) October 22, 2026 06:01

View session

Copilot finished work on behalf of Calvin S. (calvin197) October 22, 2026 06:04

Calvin S. (calvin197) force-pushed the security/node-runtime-secret-hardening branch from 06e35e0 to 8ac3817 Compare October 22, 2026 06:29

Copilot AI balanced review requested due to automatic review settings October 22, 2026 06:29

Copilot started reviewing on behalf of Calvin S. (calvin197) October 22, 2026 06:30

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Copilot started work on behalf of Calvin S. (calvin197) October 22, 2026 06:34

View session

Copilot finished work on behalf of Calvin S. (calvin197) October 22, 2026 06:36

Cameron Meissner (cameronmeissner) reviewed Oct 22, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 22, 2026 23:45

Copilot started reviewing on behalf of Calvin S. (calvin197) October 22, 2026 23:45

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Conversation

Calvin S. (calvin197) commented Oct 14, 2026

Uh oh!

github-actions Bot commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Windows Unit Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

github-actions Bot commented Oct 22, 2026

Uh oh!

Cameron Meissner (cameronmeissner) Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

github-actions Bot commented Oct 22, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Oct 14, 2026 •

edited

Loading