Skip to content

Feat/secrets generation#2825

Open
EleniKechrioti wants to merge 5 commits intoIBM:mainfrom
EleniKechrioti:feat/secrets-generation
Open

Feat/secrets generation#2825
EleniKechrioti wants to merge 5 commits intoIBM:mainfrom
EleniKechrioti:feat/secrets-generation

Conversation

@EleniKechrioti
Copy link

Related Issue

Partially addresses #2595 . Implements US-3 (Secrets Generation CLI).


Summary

This PR completes Phase 3 of the security epic by implementing the Secrets Generation CLI tool. It satisfies all acceptance criteria for US-3, providing a secure way for developers to bootstrap their production configurations.


Type of Change

  • Bug fix
  • Feature / Enhancement
  • Documentation
  • Refactor
  • Chore (deps, CI, tooling)
  • Other (describe below)

Verification

Check Command Status
Lint suite make lint PASS
Unit tests make test PASS
Coverage ≥ 90% make coverage PASS

Checklist

  • Code formatted (make black isort pre-commit)
  • Tests added/updated for changes
  • Documentation updated (if applicable)
  • No secrets or credentials committed

Notes (optional)

Implemented with a focus on cryptographic security using the secrets module.
The test suite includes 5 unit tests covering file creation, entropy, and CLI flags (--force, --stdout).
Verified in an isolated environment to bypass local dependency issues.

@crivetimihai
Copy link
Member

Thanks for this contribution, @EleniKechrioti! Clean, well-scoped utility that fills a real gap in the setup workflow. The argparse CLI, --force safety guard, and --stdout option are all nice touches.

A few suggestions:

  1. Missing newline at end of test file - tests/scripts/test_init_secrets.py is missing a trailing newline (line 111). Minor, but linters may flag it.

  2. Test mocking pattern - The patch("argparse.Namespace", ...) + .start() pattern is a bit fragile. Consider using argparse.Namespace(...) directly:

    mock_args.return_value = argparse.Namespace(output="test.env", force=False, stdout=False)

    Simpler and avoids the nested patch().start() that doesn't get .stop()'d.

  3. File permissions - For a secrets file, it might be worth setting restrictive permissions (e.g., os.chmod(args.output, 0o600)) after writing, to prevent accidental exposure.

  4. .env.secrets in .gitignore? - Since the default output is .env.secrets, consider adding it to .gitignore if it isn't already, to prevent accidental commits.

Solid stuff overall!

@EleniKechrioti
Copy link
Author

Hi @crivetimihai, thanks for the suggestions! I've updated the PR to:

  1. Set restrictive file permissions (0o600) using os.chmod for the secrets file.
  2. Refactored the test mocking pattern to use argparse.Namespace directly, avoiding fragile .start() calls.
  3. Verified that .env.* is already covered in the existing .gitignore.
  4. Fixed all linting issues.

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.

3 participants