fix(lh-102436): prepare PyPI and Galaxy release by huides00 · Pull Request #23 · CiscoDevNet/sccfm-devkit

huides00 · 2026-08-12T08:15:50Z

https://cisco-sbg.atlassian.net/browse/LH-102436

Description

Prepares sccfm-devkit for its first supported distribution through PyPI and Ansible Galaxy. The
Python package exposes the customer-facing sccfm-cli and shared runtime library, while the
cisco.sccfm collection includes the metadata, dependencies, documentation, and changelog needed
for installation from Galaxy.

Credential handling is safer across local configuration, inventory, and command output. Public
artifacts contain only supported runtime code, while contributor commands, tests, and end-to-end
tools remain available from a source checkout.

Releases are now explicit maintainer operations: a maintainer selects the version, the wheel,
source distribution, and collection are built once, and those same verified artifacts are
published to PyPI and Galaxy.

…d usable.

Update the CLI vASA onboarding and cleanup playbooks to consume the vault_sccfm_api_token field written by change-tokens. Add regression tests that reject the legacy token reference before Jenkins reaches the CLI E2E suite.

Scoombe · 2026-08-13T16:09:39Z

[P1 Security] A write-capable deploy key is exposed for almost the entire release job.
The SSH key is installed during checkout, before Poetry, dependencies, repository scripts, tests, documentation generators, and build tools run. Any compromised dependency or repository code could extract the key and push arbitrary commits or tags.
This also occurs in the generated-docs workflow, where the PR removes the previous release-bot environment boundary.
CI checkout and key · push operation · generated-docs workflow
The build and verification should run without write credentials. Acquire a short-lived GitHub App token—or narrowly scoped credential—only in the final push job/step, with persisted checkout credentials disabled.

[P1 Security] PyPI publishing executes a mutable third-party branch with the production API token.
pypa/gh-action-pypi-publish@release/v1 resolves to a branch, not an immutable commit. Whoever can update that branch controls code receiving PYPI_API_TOKEN.
PyPI publish step
Pin the action to a reviewed full commit SHA. Prefer PyPI trusted publishing with OIDC over a long-lived API token.

[P2 Reliability/Security] An interrupted config update destroys the credential file.
_rewrite() truncates the existing file before serializing the replacement. I injected a write failure and the original valid configuration became:
b'{"profiles":'
This can lose every configured profile following disk exhaustion, serialization failure, interruption, or concurrent access.
Non-atomic rewrite
Serialize first, write and fsync a private temporary file through the validated parent descriptor, then atomically replace the destination. Add locking if concurrent writers are supported.

[P2 Compatibility] Valid custom config paths under macOS /tmp and /var are rejected.
macOS defines /tmp -> private/tmp and /var -> private/var. The new ancestor validation rejects those fixed system aliases. I reproduced:
ValueError: Configuration directory path must not contain symbolic links: /tmp
This is also inconsistent with the setup transaction code, which explicitly normalizes macOS’s /var alias.
Ancestor validation
Apply the same narrowly scoped platform normalization used by setup_tokens, while continuing to reject user-controlled symlink ancestors.

[P2 Completeness] Newly generated vault passwords fall outside the credential transaction.
Both headless and interactive setup create .vault_pass before the transaction begins. If token loading or merging fails, the password remains even though setup reports failure. I reproduced this with an injected merge failure; .vault_pass remained at mode 0600.
Headless ordering · interactive ordering
Start the transaction before creating the password and reading/merging tokens, or explicitly remove a newly created password on failure.

[P2 Release completeness] The principal release path is never exercised before merge.
prepare-release contains more than 400 lines of versioning, artifact, tag, and push orchestration, but is restricted to pushes to main. GitHub reports both prepare-release and create-draft-release as skipped on this PR.
Job condition

Scoombe

Deep review of the exact current head. The implementation has strong test and artifact-verification coverage, but I found security, reliability, compatibility, and release-completeness issues that should be addressed before merge. I left actionable comments at the relevant lines. Validation performed locally: 1,535 tests, Black, isort, mypy, Poetry metadata/lock checks, wheel/sdist/collection verification, and clean-controller installation all passed.

Scoombe · 2026-08-13T16:25:03Z

+      bundle_name: ${{ steps.source.outputs.bundle_name || steps.version.outputs.bundle_name }}
    steps:
-      - name: Checkout
+      - name: Checkout main


[P1] Do not expose a write credential to the build and test process

This checkout still supplies SCCFM_CI_DEPLOY_KEY, and actions/checkout persists that SSH credential by default. Every later step—including pip/pipx installation, poetry install, repository scripts/tests, documentation generation, and artifact creation—therefore runs while a key capable of pushing main and tags is available. A compromised dependency or source step can reuse or exfiltrate it even though the job declares contents: read. Please check out without a write credential (persist-credentials: false), run all build and verification work without it, and acquire a short-lived GitHub App token or inject the key only in an isolated final push step/job.

Addressed in abc5c07. Every checkout now uses persist-credentials: false and no checkout receives the deploy key. SCCFM_CI_DEPLOY_KEY is scoped only to the final atomic push step, written with mode 0600 under RUNNER_TEMP, unset before subprocesses, used with strict GitHub SSH host-key verification, and removed by an exit trap. Dependency installation, builds, tests, documentation generation, and artifact verification therefore run without the write credential.

Scoombe · 2026-08-13T16:25:03Z

+  prepare-release:
    needs: lint-and-test
-    if: github.ref == 'refs/heads/main'
+    if: github.event_name == 'push' && github.ref == 'refs/heads/main'


[P2] Exercise release preparation before merging

This condition skips the new version-bump, recovery, manifest, tag, atomic-push, and draft-release orchestration on every pull request. A green PR therefore does not validate the most consequential shell branches until they run on main with production write access. Please extract the orchestration into testable scripts and run a no-push dry-run on PRs; adding actionlint and shellcheck coverage would catch workflow and shell defects before merge.

Addressed in abc5c07. Pull requests now run prepare-release as a credential-free rehearsal using the same preparation job as production. It exercises version inference and bumping, Ansible synchronization, documentation generation, all three artifact builds and verifiers, source gates, the local release commit, manifest creation, and the annotated tag, then stops before upload, push, or release creation. CI also runs pinned Actionlint 1.7.12 with ShellCheck 0.11.0.1, and structural regressions cover the remote-only push and draft branches.

Scoombe · 2026-08-13T16:25:04Z

+
+      - name: Publish exact Python artifacts
+        if: steps.pypi.outputs.publish == 'true'
+        uses: pypa/gh-action-pypi-publish@release/v1


[P1] Pin publishing code to an immutable commit

release/v1 is a mutable branch, and this action receives the production PYPI_API_TOKEN. An upstream branch change would execute with that credential. Please pin the action to a reviewed full commit SHA and update it through a controlled dependency-update process. Prefer PyPI trusted publishing with OIDC as well, so this workflow does not need a long-lived API token.

Addressed in abc5c07. The publisher is pinned to the reviewed full commit dc37677b2e1c63e2034f94d8a5b11f265b73ba33, and a regression test rejects the mutable release/v1 reference. Trusted Publishing remains a separate operational improvement; this patch preserves the repository-token setup while removing the mutable-action risk.

Scoombe · 2026-08-13T16:25:04Z

+        descriptor = handle.fileno()
+        self._ensure_regular_descriptor(descriptor)
+        handle.seek(0)
+        os.ftruncate(descriptor, 0)


[P2] Preserve the existing config until the replacement is durable

The file is truncated before JSON serialization completes. A write/serialization failure, interruption, or full disk leaves every stored profile corrupted; an injected failure after this line produced only {"profiles":. Please serialize first, write and fsync a mode-0600 temporary file through the validated parent descriptor, atomically replace the destination, and fsync the parent directory. Add locking if concurrent writers are supported.

Addressed in abc5c07. Saves now create a same-directory O_EXCL temporary file at mode 0600 through the validated parent descriptor, serialize and flush it, fsync the file, revalidate the destination and parent, replace descriptor-relatively, and fsync the parent directory. Pre-replacement failures clean the temporary file and preserve the installed bytes. Regression tests inject serialization, partial-write, and file-fsync failures. Locking was not added because concurrent writers are not an advertised contract.

Scoombe · 2026-08-13T16:25:04Z

+                mode = parent.lstat().st_mode
+            except FileNotFoundError:
+                continue
+            if stat.S_ISLNK(mode):


[P2] Allow the fixed macOS /tmp and /var aliases

On macOS, /tmp and /var are system symlinks to /private/tmp and /private/var, so a valid --config-path /tmp/... or SCCFM_CONFIG=/var/... now fails with this error. I reproduced the rejection locally. The setup transaction already has narrowly scoped _platform_normalized_path() handling for these aliases; apply the same normalization here while continuing to reject user-controlled symlink ancestors.

Scoombe · 2026-08-13T16:25:04Z

+    selected = _saved_token(name=name, region=region, token=api_token)
+
    # ── Vault password ───────────────────────────────────────────
    vault_pass_path = _ensure_vault_pass_headless(examples_path, vault_password)


[P2] Create the vault password inside the credential transaction

This can create .vault_pass before _credential_transaction starts. If _merge_token or any other pre-transaction operation then fails, setup reports failure but leaves the newly generated secret behind. I reproduced that outcome with an injected merge failure; the new mode-0600 file remained. Start the transaction before password creation and token loading/merging, or explicitly roll back a password created by this invocation. The interactive path has the same ordering.

Scoombe · 2026-08-13T16:25:04Z

        github.event.workflow_run.head_branch == 'main'
      )
    runs-on: ubuntu-latest
-    environment: release-bot


[P1] Keep the protected environment around this write path

Removing release-bot also removes its secret scoping and any configured deployment protection or approval rules, while this job still obtains a deploy key and pushes directly to main. Please retain the protected environment and, independently, avoid making the write credential available during dependency installation and documentation generation; inject it only for the final push.

The credential-exposure portion is addressed in abc5c07: generated-docs now has contents: read, uses a tokenless checkout, and receives the deploy key only in the final guarded push step with strict host-key checking and cleanup. We intentionally did not restore environment: release-bot because this repository has no configured GitHub environments and the maintainer selected repository-level secrets. Adding only the YAML key would auto-create an unprotected environment without approval rules or environment-scoped secrets, so it would not add protection. If a protected environment is configured later, the deploy key should move into it.

Preserve stored profiles through failed writes, support macOS path aliases, and roll back newly created vault passwords when setup fails. Keep release credentials out of build steps, pin the PyPI publisher, and rehearse the production preparation path on pull requests with workflow linting.

Install Gitleaks from the module path declared by v8.30.1 so release preparation succeeds on GitHub-hosted runners. Add a regression check that rejects the renamed repository path.

Merge the profile-based credential workflow from main into the release branch while preserving the PyPI and Ansible packaging paths. Align environment setup, generated documentation, and paired package requirements with version 0.39.0 so both changes can ship together.

Merge the latest main branch and regenerate the CLI manual pages from the combined source. This keeps the version 0.39.0 documentation current without changing CLI behavior.

Modernize type annotations, make network-object lookup arguments keyword-only, and preserve the documented empty-string response fallback. Relocate the service tests to the core test suite so the consistency check recognizes coverage across the CLI and Ansible call sites.

Apply the documented empty-string fallback and keyword-only lookup contract to network groups. Update the Ansible call site and service tests so the PR consistency check evaluates the complete object-management surface without warnings.

Align module documentation and import layout with ansible-test requirements, and keep plugin dependency imports safe during isolated sanity inspection. Correct argument specifications and fallback definitions, then cover missing controller dependencies with regression tests.

Read the provisioned vASA password from Ansible Vault instead of the process environment. Document the variable in the example vault and add regression coverage for the onboarding playbook.

Treat 0.39.1 as the first public Ansible release while retaining 0.39.0 as the immutable pre-release seed. Update validation and release guidance so later releases must preserve 0.39.1 and add new changelog history.

Invoke object and group lookup callbacks with their declared name and UID keyword arguments in the Ansible and CLI helpers. This restores updates by name and check-mode preflights after the service methods became keyword-only. Add strict regression coverage for both identifier paths so positional calls cannot return unnoticed.

github-actions · 2026-08-18T19:26:49Z

Consistency Check

No consistency issues found.

Checker output

✓ No issues found across 147 file(s).

huides00 added 16 commits August 11, 2026 12:14

fix(lh-102436):Local credentials cannot enter the Galaxy artifact

a5e3cc1

fix(lh-102436): Inventory plugin remove exports of the API token

b58da43

fix(lh-102436): hide smart licensing token

0396937

fix(lh-102436): install resolves a broken SDK

2b3ee4b

fix(lh-102436): hide api-token

bb75c2e

fix(lh-102436): unresolved runtime dependency

d7921f6

fix(lh-102436): query correct distribution name

1d6d303

fix(lh-102436): remove tests and e2e files from pypi wheel

684b70e

fix(lh-102436): make the Galaxy artifact independently installable an…

d69bea6

…d usable.

fix(lh-102436): Galaxy metadata/runtime issues

a04e3cd

fix(lh-102436): poetry run in venv

3c7b987

fix(lh-102436): stabilizes the final package metadata and README

7338f0d

fix(lh-102436): add manual build-once release workflow

e58ecbb

fix(lh-102436): harden release and credential workflows

226fb91

fix(lh-102436): use current vault key in CLI E2E

5765498

Update the CLI vASA onboarding and cleanup playbooks to consume the vault_sccfm_api_token field written by change-tokens. Add regression tests that reject the legacy token reference before Jenkins reaches the CLI E2E suite.

fix(lh-102436): close remaining release blockers

f2e8f47

huides00 marked this pull request as ready for review August 13, 2026 08:58

huides00 requested review from Scoombe, afercal and siddhuwarrier as code owners August 13, 2026 08:58

huides00 added 3 commits August 13, 2026 14:15

fix(lh-102436): use repository secrets for publishing

ee0e8c4

fix(lh-102436): fix doc styling

3d3e50d

fix(lh-102436): proper release

081eff8

huides00 changed the title ~~Lh 102436/prepare pypi and ansible release~~ fix(lh-102436): prepare PyPI and Galaxy release Aug 13, 2026

Scoombe reviewed Aug 13, 2026

View reviewed changes

huides00 added 4 commits August 17, 2026 14:04

fix(lh-102436): correct gitleaks module path

3aa71bb

Install Gitleaks from the module path declared by v8.30.1 so release preparation succeeds on GitHub-hosted runners. Add a regression check that rejects the renamed repository path.

fix(lh-102436): resolve generated documentation conflicts

1f408b2

Merge the latest main branch and regenerate the CLI manual pages from the combined source. This keeps the version 0.39.0 documentation current without changing CLI behavior.

huides00 added 5 commits August 18, 2026 11:27

fix(lh-102436): vault ansible e2e device password

10072f0

Read the provisioned vASA password from Ansible Vault instead of the process environment. Document the variable in the example vault and add regression coverage for the onboarding playbook.

fix(lh-102436): prepare first ansible release metadata

6059b6a

Treat 0.39.1 as the first public Ansible release while retaining 0.39.0 as the immutable pre-release seed. Update validation and release guidance so later releases must preserve 0.39.1 and add new changelog history.

Scoombe approved these changes Aug 18, 2026

View reviewed changes

huides00 merged commit e5aa174 into main Aug 19, 2026
5 checks passed

Uh oh!

Conversation

huides00 commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Scoombe commented Aug 13, 2026

Uh oh!

Scoombe left a comment

Choose a reason for hiding this comment

Uh oh!

Scoombe Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huides00 Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Scoombe Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huides00 Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Scoombe Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huides00 Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Scoombe Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huides00 Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Scoombe Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Scoombe Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Scoombe Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huides00 Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 18, 2026

Consistency Check

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

huides00 commented Aug 12, 2026 •

edited

Loading

Scoombe Aug 13, 2026 •

edited

Loading

Scoombe Aug 13, 2026 •

edited

Loading

Scoombe Aug 13, 2026 •

edited

Loading

Scoombe Aug 13, 2026 •

edited

Loading

Scoombe Aug 13, 2026 •

edited

Loading

Scoombe Aug 13, 2026 •

edited

Loading

Scoombe Aug 13, 2026 •

edited

Loading