Skip to content

Clear initial admin password after provisioning - #266

Open
itsmiso-ai wants to merge 1 commit into
mainfrom
foreman/wl-misospace-kubetix-251/issue-251
Open

Clear initial admin password after provisioning#266
itsmiso-ai wants to merge 1 commit into
mainfrom
foreman/wl-misospace-kubetix-251/issue-251

Conversation

@itsmiso-ai

Copy link
Copy Markdown
Contributor

What

Cleans up the INITIAL_ADMIN_PASSWORD environment variable and zeroes the local _admin_password variable after the initial admin account is provisioned in kubetix-api/main.py. Adds a unit test asserting the env var is no longer present in os.environ post-startup…

Fixes #251

Opened by foreman on review GO (workload wl-misospace-kubetix-251).

Remove INITIAL_ADMIN_PASSWORD from os.environ and clear the local value once startup admin provisioning completes, including provisioning failures. Add a regression test for the existing-admin path and format the API module.

Fixes #251

Signed-off-by: Saffron <263493777+itsmiso-ai@users.noreply.github.com>
@itsmiso-ai
itsmiso-ai requested a review from joryirving as a code owner July 25, 2026 15:34

@its-saffron its-saffron Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — routed smart (risk match: secret_handling_changes)

Recommendation: Approve

This PR correctly implements the security fix described in issue PR 251. The INITIAL_ADMIN_PASSWORD environment variable is now removed from os.environ and the local variable is zeroed out in a finally block immediately after the admin provisioning logic completes — whether the admin was created or already existed.

Change-by-Change Findings

kubetix-api/main.py

  • Wrapped the entire admin-provisioning block in a try/finally.
  • finally unconditionally calls _os.environ.pop("INITIAL_ADMIN_PASSWORD", None) and sets _admin_password = "".
  • The pop uses None as the default, which is safe and idiomatic.
  • The clearing happens on both paths: first startup (admin created) and subsequent restarts (admin already exists), satisfying the issue's acceptance criteria.
  • No secret values appear anywhere in the diff — only the env var name INITIAL_ADMIN_PASSWORD and a test placeholder "bootstrap-secret" are visible.

tests/unit/test_initial_admin_password.py (new file)

  • Tests the scenario where the admin already exists (ExistingAdminQuery.first() returns an object), entering the "admin exists, skip creation" branch.
  • Asserts INITIAL_ADMIN_PASSWORD is absent from os.environ both inside the lifespan context and after it exits.
  • Mocks database.SessionLocal and cleanup.run_grant_cleanup_loop to isolate the lifespan logic.
  • The test implicitly covers the "create new admin" path too, because the finally block clears the env var regardless of which branch inside try executed.

Required Checks

verify secrets are not logged or exposed in diffs

Verified. No actual secret values appear in the diff. The test uses only a placeholder string "bootstrap-secret". The production code references only the env var name INITIAL_ADMIN_PASSWORD and sets the local variable to an empty string "".

check secret rotation impact

Does not apply. INITIAL_ADMIN_PASSWORD is a first-boot bootstrap credential, not a rotatable operational secret. The Helm chart's secret rotation runbook (docs/runbooks/03-secret-rotation.md) does not list it among rotation-eligible credentials. On subsequent restarts, the code reads os.environ once, finds the admin already exists, and clears the env var again — the behavior is idempotent and rotation-safe.

Standards Compliance

  • AGENTS.md states: "The API creates one admin account (admin@kubetix.local) with that password on first startup, then never exposes or logs it." This PR enforces that guarantee at runtime by removing the env var immediately after use.
  • The finally-block pattern is the correct defensive idiom: cleanup runs even if an exception occurs during provisioning.
  • The test follows existing repository patterns (async lifespan runner, monkeypatch, fake DB session).

Linked Issue Fit

Issue PR 251 acceptance criteria: "After the admin user is verified to exist (or created), clear the env var from os.environ and zero the local variable."

The implementation clears os.environ via pop and zeros _admin_password in a finally block that runs after both the "admin exists" and "admin created" branches. ✅ Fully satisfied.

CI Check Results

All 9 CI checks passed, including test, security, and lint — confirming the new test and the clearing logic are correct and introduce no regressions.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P1] INITIAL_ADMIN_PASSWORD lingers in process environment after admin provisioning

1 participant