Skip to content

Implement remote signing providers with rotation telemetry#61

Closed
DevOpsMadDog wants to merge 2 commits into
mainfrom
codex/fix-and-split-decisionfactory_alignment.md-kl7u8l
Closed

Implement remote signing providers with rotation telemetry#61
DevOpsMadDog wants to merge 2 commits into
mainfrom
codex/fix-and-split-decisionfactory_alignment.md-kl7u8l

Conversation

@DevOpsMadDog
Copy link
Copy Markdown
Owner

Summary

  • replace the placeholder key providers with functional AWS KMS and Azure Key Vault integrations, rotation helpers, and health evaluation utilities
  • extend configuration, overlays, and the CLI to surface signing provider selection, rotation SLAs, and remote connection details alongside Prometheus gauges for key health
  • add regression suites that exercise the remote provider stubs, CLI overrides, and overlay validation for signing controls

Testing

  • pytest tests/test_key_management.py tests/test_cli.py tests/test_overlay_configuration.py tests/test_crypto_signing.py

https://chatgpt.com/codex/tasks/task_e_68e4ee4183748329b8d6bba14a4ab0c4

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

Comment on lines +366 to +370
def sign(self, payload: bytes) -> bytes:
response = self._crypto_client.sign(payload) # type: ignore[call-arg]
signature = _extract_signature(response)
if signature is None:
raise RuntimeError("Azure Key Vault did not return a signature")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Use CryptographyClient.sign without required algorithm

The Azure Key Vault provider’s sign implementation calls self._crypto_client.sign(payload) without supplying a SignatureAlgorithm or digest. The Azure SDK requires two arguments (algorithm and the hashed payload) and will raise TypeError: sign() missing required positional argument as soon as an Azure-backed provider is exercised. This effectively makes the Azure signing backend unusable in production.

Useful? React with 👍 / 👎.

Comment on lines +187 to +199
if not cve_ids:
return {}

now = _normalize_datetime(datetime.now(timezone.utc))

stmt = (
select(KevWaiverModel)
.where(
KevWaiverModel.cve_id.in_(list(cve_ids)),
KevWaiverModel.is_active.is_(True),
KevWaiverModel.expires_at >= now,
)
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Persist timezone-naive timestamps into tz-aware waiver columns

Policy endpoints normalise all datetimes via _normalize_datetime, which strips the timezone info (datetime.now(timezone.utc) → naive) before comparing or persisting to the KEV waiver model. However, the waiver ORM columns are declared as DateTime(timezone=True) in KevFindingWaiver, so passing naive values will trigger SQLAlchemy’s "naive datetime is disallowed when time zone support is active" errors on Postgres and block waiver creation or querying. Keep timestamps timezone-aware when interacting with these columns.

Useful? React with 👍 / 👎.

@devin-ai-integration
Copy link
Copy Markdown
Contributor

Closing as part of PR consolidation. Useful changes have been cherry-picked into PR #240.

DevOpsMadDog added a commit that referenced this pull request May 5, 2026
…4 at HEAD e9014b0

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant