Skip to content

[Aikido] Prevent arbitrary command execution via unskipped !exec YAML tags in secret enumeration - #2972

Open
aikido-autofix[bot] wants to merge 1 commit into
mainfrom
fix/aikido-security-code-audit-93654111-1qmr
Open

[Aikido] Prevent arbitrary command execution via unskipped !exec YAML tags in secret enumeration#2972
aikido-autofix[bot] wants to merge 1 commit into
mainfrom
fix/aikido-security-code-audit-93654111-1qmr

Conversation

@aikido-autofix

Copy link
Copy Markdown

what

This patch addresses an arbitrary command execution vulnerability in secret enumeration and list inventory operations by ensuring the !exec YAML tag is properly skipped when credentials are unavailable. The !exec function was missing from credential-free skip lists, allowing potentially malicious YAML configurations to execute arbitrary commands during enumeration. The fix adds AtmosYamlFuncExec to all relevant skip lists across cmd/list/utils.go, cmd/secret/shared.go, cmd/secret/shared_test.go, pkg/ai/tools/atmos/secret_list.go, and pkg/ai/tools/atmos/secret_list_test.go, ensuring consistent security posture across all secret and inventory listing operations.

why

references

…to prevent arbitrary command execution during secret enumeration and list inventory operations.
@aikido-autofix
aikido-autofix Bot requested a review from a team as a code owner August 20, 2026 22:10
@atmos-pro

atmos-pro Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Tip

Atmos Pro  

No affected stacks workflow was detected for this pull request.
If this is expected, no action is needed.
Learn More. Ask AI.

@github-actions github-actions Bot added the size/s Small size PR label Aug 20, 2026
@github-actions

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

@osterman Erik Osterman (Cloud Posse) (osterman) added the patch A minor, backward compatible change label Aug 20, 2026
@osterman

Copy link
Copy Markdown
Member

CodeRabbit (@coderabbitai) isn't the entire point of !exec to execute arbitrary commands? What am I not getting.

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

for file in cmd/list/utils.go cmd/secret/shared.go cmd/secret/shared_test.go pkg/ai/tools/atmos/secret_list.go pkg/ai/tools/atmos/secret_list_test.go; do
  if [ -f "$file" ]; then
    echo "===== $file ====="
    rg -n -C 5 'AtmosYamlFuncExec|skip|Skip|Process|yaml' "$file" || true
  fi
done

echo "===== Definitions and call sites ====="
rg -n -C 4 'AtmosYamlFuncExec|func.*[Ss]kip|[Ss]kip.*func|skip.*functions' \
  -g '*.go' . || true

Length of output: 175509


@osterman You are correct. !exec exists to run commands when Atmos processes YAML functions.

The issue is narrower than I stated. Secret and inventory enumeration are intended to inspect declarations without resolving credentials or evaluating side-effecting YAML functions. Those paths already skip !secret, !store, and Terraform lookup tags. They did not skip !exec, so an inspection operation could execute it as an unintended side effect.

This patch does not disable !exec for normal Atmos processing. It skips !exec only in the credential-free enumeration paths.

My “arbitrary command execution vulnerability” wording was too broad. The relevant concern is unintended command execution during a non-execution operation.

You are interacting with an AI system.

@osterman

Copy link
Copy Markdown
Member

CodeRabbit (@coderabbitai) I agree with the stated concern, but this is a policy based concern. Controlling when !exec runs is different from evaluating other functions that need auth. There we gracefully degrade if it's not available. With !exec could legitimately be reasons it needs to execute to inject the config. E.g. injecting the very auth config needed to perform auth.

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@osterman Agreed. This is an execution-policy decision, not a credential-availability safeguard.

!secret, !store, and similar functions fail or degrade when credentials are unavailable. !exec can establish the configuration or credentials that those functions need. A blanket AtmosYamlFuncExec skip can therefore break valid bootstrap flows.

The patch is appropriate only if list and secret-enumeration operations explicitly guarantee that they never evaluate !exec. That guarantee should be a documented product contract. Otherwise, the behavior needs a separate policy mechanism, such as an explicit safe-enumeration mode or an opt-in execution setting.

I do not consider the unconditional skip a generally safe correctness change without that contract. 🐇


✏️ Learnings added
Learnt from: osterman
URL: https://github.com/cloudposse/atmos/pull/2972

Timestamp: 2026-08-21T02:27:17.367Z
Learning: In Atmos YAML evaluation, `!exec` can generate authentication configuration or credentials required by later functions. Skipping `AtmosYamlFuncExec` during secret or inventory enumeration is an execution-policy change, not equivalent to graceful degradation when credential-dependent functions cannot authenticate. A blanket skip can break valid credential-bootstrap flows unless the operation explicitly contracts never to evaluate `!exec`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

@osterman

Copy link
Copy Markdown
Member

CodeRabbit (@coderabbitai) give me an example of how/where list and secret-enumeration operations explicitly could use !exec. Maybe it makes sense to disable, but it's too abstract/opaque to comprehend without an example.

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

for file in cmd/list/utils.go cmd/secret/shared.go pkg/ai/tools/atmos/secret_list.go; do
  echo "===== OUTLINE: $file ====="
  ast-grep outline "$file" --items all --type function --view expanded || true
  echo
  echo "===== MATCHES: $file ====="
  rg -n -C 8 'AtmosYamlFuncExec|skipCustom|skip.*Func|Process.*YAML|ProcessYAML|yaml.*Process|secret|inventory' "$file" || true
  echo
done

echo "===== YAML FUNCTION DEFINITIONS / EVALUATION CALLS ====="
rg -n -C 5 'AtmosYamlFuncExec|func.*Exec|!exec|Process.*Yaml|Process.*YAML|Process.*Config' -g '*.go' . || true

Length of output: 200949


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "===== Target-file matches ====="
rg -n -C 12 'AtmosYamlFuncExec|ProcessCustomYamlTags|ExecuteDescribeComponent|skip' \
  cmd/list/utils.go \
  cmd/secret/shared.go \
  pkg/ai/tools/atmos/secret_list.go \
  cmd/secret/shared_test.go \
  pkg/ai/tools/atmos/secret_list_test.go || true

echo "===== Target-file function map ====="
for file in cmd/list/utils.go cmd/secret/shared.go pkg/ai/tools/atmos/secret_list.go; do
  echo "--- $file ---"
  ast-grep outline "$file" --items all --type function --view expanded || true
done

echo "===== Existing !exec examples in relevant areas ====="
rg -n -C 8 '!exec|AtmosYamlFuncExec' \
  cmd/list cmd/secret pkg/ai/tools/atmos tests/fixtures \
  -g '*.go' -g '*.yaml' -g '*.yml' || true

echo "===== YAML tag handler ====="
rg -n -C 10 'func ProcessCustomYamlTags\b|func.*AtmosYamlFuncExec\b|AtmosYamlFuncExec\s*=' \
  internal pkg -g '*.go' || true

Length of output: 45191


@osterman I traced the paths. The concrete case is that secret list processes the entire component configuration before it extracts secrets.vars.

For example, a stack can contain:

components:
  terraform:
    app:
      vars:
        aws_profile: !exec ./scripts/select-aws-profile.sh
      secrets:
        vars:
          API_TOKEN:
            sops:
              file: secrets/app.yaml
              path: /api_token

Without this patch, atmos secret list --stack <stack> --component app reaches ExecuteDescribeComponent with ProcessYamlFunctions: true. It evaluates vars.aws_profile and runs ./scripts/select-aws-profile.sh. The list command then ignores vars.aws_profile and only reads the API_TOKEN declaration.

This is not hypothetical evaluation behavior. The repository has fixtures with !exec in component env and vars fields. The secret-list path processes those fields before it filters to secrets.vars.

However, I found no existing fixture or code path that requires !exec to enumerate secret declarations. In the example above, !exec is an incidental effect of processing the full component. It does not contribute to the list result.

Your bootstrap case could be valid for a secret operation that resolves a secret provider, for example if a provider configuration obtains a credential through !exec. But this implementation does not support that case consistently:

  • secret list without --verify skips !exec.
  • loadService() also skips !exec after it creates an AuthManager.
  • Therefore, secret list --verify and other loadService() users cannot use !exec for provider bootstrap either.

So the important distinction is:

  1. list inventory can legitimately evaluate !exec today when an auth manager exists.
  2. Secret enumeration has no demonstrated need to evaluate it.
  3. This patch makes !exec unavailable for all secret-service configuration loading, not only credential-free enumeration.

That makes the change reasonable if the intended contract is “secret declarations must be static.” It is too broad if secret provider configuration is expected to support command-based bootstrap.

You are interacting with an AI system.

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

Labels

patch A minor, backward compatible change size/s Small size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant