Skip to content

fix(pyats): prevent credential exposure in PyATS archives (#689)#698

Merged
oboehmer merged 1 commit intomainfrom
fix/689-quick-fix
Mar 24, 2026
Merged

fix(pyats): prevent credential exposure in PyATS archives (#689)#698
oboehmer merged 1 commit intomainfrom
fix/689-quick-fix

Conversation

@oboehmer
Copy link
Copy Markdown
Collaborator

@oboehmer oboehmer commented Mar 22, 2026 •
edited
Loading

Description

Quick fix to prevent credential exposure in PyATS result archives. The EnvironmentDebugPlugin was writing environment variables (including passwords) to env.txt files within PyATS archives.

This is an expedited fix to address the security issue before UAT. A more comprehensive PR (#697) is in progress that includes this fix along with additional cleanup for #570.

Closes

Related Issue(s)

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring / Technical debt (internal improvements with no user-facing changes)
  • Documentation update
  • Chore (build process, CI, tooling, dependencies)
  • Other (please describe):

Test Framework Affected

  • PyATS
  • Robot Framework
  • Both
  • N/A (not test-framework specific)

Network as Code (NaC) Architecture Affected

  • ACI (APIC)
  • NDO (Nexus Dashboard Orchestrator)
  • NDFC / VXLAN-EVPN (Nexus Dashboard Fabric Controller)
  • Catalyst SD-WAN (SDWAN Manager / vManage)
  • Catalyst Center (DNA Center)
  • ISE (Identity Services Engine)
  • FMC (Firepower Management Center)
  • Meraki (Cloud-managed)
  • NX-OS (Nexus Direct-to-Device)
  • IOS-XE (Direct-to-Device)
  • IOS-XR (Direct-to-Device)
  • Hyperfabric
  • All architectures
  • N/A (architecture-agnostic)

Platform Tested

  • macOS (version tested: )
  • Linux (distro/version tested: )

Key Changes

  • Disable EnvironmentDebugPlugin in PyATS subprocess runner configuration to prevent env vars from being written to archives
  • Add E2E regression test that scans all output artifacts (including ZIP contents) for credential sentinel values

Testing Done

  • Unit tests added/updated
  • Integration tests performed
  • Manual testing performed:
    • PyATS tests executed successfully
    • Robot Framework tests executed successfully
    • D2D/SSH tests executed successfully (if applicable)
    • HTML reports generated correctly
  • All existing tests pass (pytest / pre-commit run -a)

Test Commands Used

without the fix in subprocess_runner, I see the new test fail:

E       AssertionError: Credential sentinel found in output artifacts:
E           - /private/var/folders/_f/vfw2shn13kqfdqskxs8s70qh0000gn/T/pytest-of-oboehmer/pytest-72/popen-gw13/e2e_pyats_cc0/pyats_results/api/env.txt
E           - /private/var/folders/_f/vfw2shn13kqfdqskxs8s70qh0000gn/T/pytest-of-oboehmer/pytest-72/popen-gw13/e2e_pyats_cc0/pyats_results/d2d/sd-dc-c8kv-02/env.txt
E           - /private/var/folders/_f/vfw2shn13kqfdqskxs8s70qh0000gn/T/pytest-of-oboehmer/pytest-72/popen-gw13/e2e_pyats_cc0/pyats_results/d2d/sd-dc-c8kv-01/env.txt
E       assert not ['/private/var/folders/_f/vfw2shn13kqfdqskxs8s70qh0000gn/T/pytest-of-oboehmer/pytest-72/popen-gw13/e2e_pyats_cc0/pyats...kqfdqskxs8s70qh0000gn/T/pytest-of-oboehmer/pytest-72/popen-gw13/e2e_pyats_cc0/pyats_results/d2d/sd-dc-c8kv-01/env.txt']

tests/e2e/test_e2e_scenarios.py:987: AssertionError
[...]
======================================================================== short test summary info =========================================================================
FAILED tests/e2e/test_e2e_scenarios.py::TestE2EPyatsApiOnly::test_no_credentials_in_output_artifacts - AssertionError: Credential sentinel found in output artifacts:
FAILED tests/e2e/test_e2e_scenarios.py::TestE2EVerboseWithInfo::test_no_credentials_in_output_artifacts - AssertionError: Credential sentinel found in output artifacts:
FAILED tests/e2e/test_e2e_scenarios.py::TestE2EVerbose::test_no_credentials_in_output_artifacts - AssertionError: Credential sentinel found in output artifacts:
FAILED tests/e2e/test_e2e_scenarios.py::TestE2EPyatsD2dOnly::test_no_credentials_in_output_artifacts - AssertionError: Credential sentinel found in output artifacts:
FAILED tests/e2e/test_e2e_scenarios.py::TestE2EAllFail::test_no_credentials_in_output_artifacts - AssertionError: Credential sentinel found in output artifacts:
FAILED tests/e2e/test_e2e_scenarios.py::TestE2ESuccess::test_no_credentials_in_output_artifacts - AssertionError: Credential sentinel found in output artifacts:
FAILED tests/e2e/test_e2e_scenarios.py::TestE2EMixedRelativeOutput::test_no_credentials_in_output_artifacts - AssertionError: Credential sentinel found in output artifacts:
FAILED tests/e2e/test_e2e_scenarios.py::TestE2EMixed::test_no_credentials_in_output_artifacts - AssertionError: Credential sentinel found in output artifacts:
FAILED tests/e2e/test_e2e_scenarios.py::TestE2EPyatsCc::test_no_credentials_in_output_artifacts - AssertionError: Credential sentinel found in output artifacts:
================================================== 9 failed, 644 passed, 270 skipped, 127 warnings in 92.33s (0:01:32) ===================================================

Checklist

  • Code follows project style guidelines (pre-commit run -a passes)
  • Self-review of code completed
  • Code is commented where necessary (especially complex logic)
  • Documentation updated (if applicable)
  • No new warnings introduced
  • Changes work on both macOS and Linux
  • CHANGELOG.md updated (if applicable)

Screenshots (if applicable)

N/A

Additional Notes

This is a minimal, targeted fix to mitigate the security issue quickly. PR #697 will supersede this with a more comprehensive solution that also addresses #570 (tech debt cleanup and code deduplication).

@oboehmer oboehmer requested a review from aitestino March 22, 2026 21:33
@oboehmer oboehmer self-assigned this Mar 22, 2026
@oboehmer oboehmer added bug Something isn't working prio: high labels Mar 22, 2026
@oboehmer
fix(pyats): prevent credential exposure in PyATS archives (#689)
07b6270 
Disable EnvironmentDebugPlugin which was writing environment variables
(including passwords) to env.txt files in PyATS result archives.

Add E2E regression test that scans all output artifacts (including ZIP
contents) for a sentinel password value to catch future credential leaks.
@oboehmer oboehmer force-pushed the fix/689-quick-fix branch from 09f0c07 to 07b6270 Compare March 23, 2026 12:27
@oboehmer oboehmer requested a review from danischm March 23, 2026 12:54
aitestino
aitestino approved these changes Mar 24, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@aitestino aitestino left a comment

Choose a reason for hiding this comment

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

LGTM — clean, minimal security fix. The plugin disable mechanism is correct and the sentinel-based E2E regression test is well-designed.

A couple of minor suggestions that can land in #697:

  • Add : str type annotation to TEST_CREDENTIAL_SENTINEL for convention consistency
  • Consider a brief YAML comment above the EnvironmentDebugPlugin: enabled: False entries explaining the rationale (#689)

Approving as-is — the expedited fix is the right call before UAT.

@aitestino
Copy link
Copy Markdown
Collaborator

aitestino commented Mar 24, 2026

Hey @oboehmer — I haven't verified this against an actual run, but while reviewing #698 I noticed that testbed_generator.py writes broker_testbed.yaml to the output directory with plaintext device credentials (IOSXE_PASSWORD), and it doesn't look like it ever gets cleaned up after D2D tests complete. So even with EnvironmentDebugPlugin disabled, the output directory would still contain a file with cleartext SSH passwords — which is the same directory users archive, upload as CI artifacts, etc.

Quick glance only — I could be wrong about the cleanup path. Worth a look?

P.S. — This comment was drafted using voice-to-text via Claude Code. If the tone comes across as overly direct or terse, please know that's just how it tends to phrase things. No offense or criticism is intended — this is purely an objective technical review of the PR. Thanks for understanding! 🙂

@oboehmer
Copy link
Copy Markdown
Collaborator Author

oboehmer commented Mar 24, 2026

Hey @oboehmer — I haven't verified this against an actual run, but while reviewing #698 I noticed that testbed_generator.py writes broker_testbed.yaml to the output directory with plaintext device credentials (IOSXE_PASSWORD), and it doesn't look like it ever gets cleaned up after D2D tests complete. So even with EnvironmentDebugPlugin disabled, the output directory would still contain a file with cleartext SSH passwords — which is the same directory users archive, upload as CI artifacts, etc.

Quick glance only — I could be wrong about the cleanup path. Worth a look?

Ouch, good catch.. Will fix this as part of #697 soon.

Merging this one as is, will address your comment in the "real" fix

@oboehmer
Copy link
Copy Markdown
Collaborator Author

oboehmer commented Mar 24, 2026

Actually: we shouldn't log testbed creds, we should save the %ENV{IOSXE_PASSWORD} value, otherwise the e2e test I added should have caught it.

@oboehmer oboehmer merged commit acb98d2 into main Mar 24, 2026
22 checks passed
@oboehmer oboehmer deleted the fix/689-quick-fix branch March 24, 2026 15:55
oboehmer added a commit that referenced this pull request Mar 24, 2026
@oboehmer
address review feedback from #698
23481f2
@oboehmer
Copy link
Copy Markdown
Collaborator Author

oboehmer commented Mar 24, 2026

Actually: we shouldn't log testbed creds, we should save the %ENV{IOSXE_PASSWORD} value, otherwise the e2e test I added should have caught it.

@aitestino , confirmed, pyats dumps the %ENV{..} string

(nac-test-2.0-UAT) OBOEHMER-M-5D76:simple-pyats oboehmer$ cat results/pyats_results/d2d/sd-dc-c8kv-02/testbed.static.yaml
testbed:
  credentials:
    default:
      password: '%ENV{IOSXE_PASSWORD}'
      username: '%ENV{IOSXE_USERNAME}'
  name: simple-pyats
[...]

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

Reviewers

@aitestino aitestino aitestino approved these changes

@danischm danischm Awaiting requested review from danischm

Assignees

@oboehmer oboehmer

Labels

bug Something isn't working prio: high

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Security: Disable pyATS EnvironmentDebugPlugin to prevent credential exposure in env.txt

2 participants

@oboehmer @aitestino