OVS_NTF 1.0: OH20-1544: Remove Secret and Notification-Signature requ…

[HenrikHL]

OH20-1544: Relax on secret and Notification-Signature

[qodo-code-review]

PR Summary by Qodo

Allow Authenticated Callbacks Without Shared Notification Secrets

✨ Enhancement 📝 Documentation 🕐 20-40 Minutes

AI Description

• Allows callback authentication to replace shared-secret HMAC verification for notification
 delivery.
• Makes signature headers conditional and permits existing subscription secrets to be removed.
• Clarifies callback activation requirements across subscription and notification schemas.

Diagram

graph TD
  S["Callback subscription"] --> A{"Callback auth?"}
  A -- Yes --> C["Authenticated callback"] --> O["Optional HMAC headers"] --> D["Deliver notification"]
  A -- No --> Q{"Shared secret?"}
  Q -- Yes --> H["Required HMAC headers"] --> D
  Q -- No --> X["Block delivery"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Model callback authentication in OpenAPI

• ➕ Enables machine-readable validation of the required security alternative.
• ➕ Makes the callback security mechanism explicit to API consumers.
• ➕ Could enforce authentication-or-secret rules through composed schemas.
• ➖ Callback authentication is currently configured outside this API's scope.
• ➖ Would introduce additional public schema and potentially implementation-specific details.
• ➖ Creates a larger compatibility and governance change than the requested relaxation.

Recommendation: Keep the PR's conditional documentation and nullable secret approach because callback authentication is configured outside the API and therefore cannot currently be validated by the schema. If callback authentication later becomes API-managed, model both security alternatives explicitly and enforce them with composed schemas.

Files changed (1) +58 / -18

Enhancement (1) +58 / -18

OVS_HUB_NTF_v1.0.0.yamlMake callback HMAC security conditional and secrets removable +58/-18

Make callback HMAC security conditional and secrets removable

• Updates the OVS Notification API contract so callback authentication can replace shared-secret HMAC verification, making signature headers conditional. The secret endpoint now accepts a required nullable property to set, replace, or remove a secret, while subscription documentation prohibits unsecured callback delivery.

ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Empty secrets make signatures forgeable ✓ Resolved 🐞 Bug ⛨ Security

Description

put-secret defines secret as nullable with only a maxLength, even though its description
requires every non-null value to be non-empty. A request containing {"secret":""} therefore
satisfies the schema and can configure HMAC with a publicly known empty key, allowing notification
signatures to be reproduced by anyone.

Code

ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[R600-605]

+                  nullable: true
+                  maxLength: 1024
                  description: |
-                    A Base64 encoded secret sent to the OVS Hub from the customer.
-                    It is used to compute the contents of the `Notification-Signature` header every time a notification is sent.
+                    A Base64-encoded shared secret used to calculate the `Notification-Signature`.
+
+                    Provide a non-null, non-empty value to set or replace the subscription secret.

Evidence

The endpoint distinguishes null removal from setting a non-empty secret, but the modified schema
imposes no lower length bound. Since an empty string is a valid string under this schema, it can
reach the HMAC-signing path as the configured key.

ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[570-576]
ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[597-609]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The secret update schema accepts an empty string even though only a non-empty value or `null` is valid, permitting signatures to use a known empty HMAC key.

## Fix Focus Areas
- ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[597-605]

## Recommended Fix
Add `minLength: 1` to the nullable `secret` property so `null` remains the explicit removal operation while empty strings are rejected.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Consumers may reject valid callbacks ✓ Resolved 🐞 Bug ≡ Correctness

Description

The Request-Id and Signature-Timestamp text still requires receivers to validate every
notification, and Notification-Signature still requires every notification to be authenticated by
computing an HMAC, despite newly allowing all three headers to be absent. On a
callback-authenticated subscription without a secret, those checks cannot be performed, so
conforming consumers can reject otherwise valid deliveries or implement incompatible behavior.

Code

ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[74]

+            When a shared secret is configured for the subscription, `Request-Id`, `Signature-Timestamp`, and `Notification-Signature` **MUST** all be included. When no shared `secret` is configured, these headers are optional and callback authentication **MUST** be used instead.

Evidence

The newly added conditional clauses permit all three headers to be absent without a secret, but
preceding text in the same parameter definitions retains unconditional MUST-level validation
requirements. The no-secret delivery mode therefore cannot satisfy all normative statements in the
operation.

ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[65-90]
ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[96-130]
ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[14-27]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The notification operation allows signature headers to be omitted for callback-authenticated subscriptions, while earlier normative text still tells receivers to perform signature-based validation for every notification.

## Fix Focus Areas
- ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[65-90]
- ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[96-130]

## Recommended Fix
Qualify the uniqueness, freshness, and HMAC-verification requirements so they apply only when a shared secret is configured and the signature headers are supplied; direct consumers using callback authentication to validate that mechanism instead.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

Review mode: ⚖️ Balanced: This changes a security-sensitive authentication and signature contract in the OpenAPI schema, creating real behavioral and interoperability risk despite being confined to one file.

Tip of the day

💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 24d970a ⚖️ Balanced

Results up to commit 26e093b ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Empty secrets make signatures forgeable ✓ Resolved 🐞 Bug ⛨ Security

Description

put-secret defines secret as nullable with only a maxLength, even though its description
requires every non-null value to be non-empty. A request containing {"secret":""} therefore
satisfies the schema and can configure HMAC with a publicly known empty key, allowing notification
signatures to be reproduced by anyone.

Code

ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[R600-605]

+                  nullable: true
+                  maxLength: 1024
                  description: |
-                    A Base64 encoded secret sent to the OVS Hub from the customer.
-                    It is used to compute the contents of the `Notification-Signature` header every time a notification is sent.
+                    A Base64-encoded shared secret used to calculate the `Notification-Signature`.
+
+                    Provide a non-null, non-empty value to set or replace the subscription secret.

Evidence

The endpoint distinguishes null removal from setting a non-empty secret, but the modified schema
imposes no lower length bound. Since an empty string is a valid string under this schema, it can
reach the HMAC-signing path as the configured key.

ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[570-576]
ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[597-609]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The secret update schema accepts an empty string even though only a non-empty value or `null` is valid, permitting signatures to use a known empty HMAC key.

## Fix Focus Areas
- ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[597-605]

## Recommended Fix
Add `minLength: 1` to the nullable `secret` property so `null` remains the explicit removal operation while empty strings are rejected.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Consumers may reject valid callbacks ✓ Resolved 🐞 Bug ≡ Correctness

Description

The Request-Id and Signature-Timestamp text still requires receivers to validate every
notification, and Notification-Signature still requires every notification to be authenticated by
computing an HMAC, despite newly allowing all three headers to be absent. On a
callback-authenticated subscription without a secret, those checks cannot be performed, so
conforming consumers can reject otherwise valid deliveries or implement incompatible behavior.

Code

ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[74]

+            When a shared secret is configured for the subscription, `Request-Id`, `Signature-Timestamp`, and `Notification-Signature` **MUST** all be included. When no shared `secret` is configured, these headers are optional and callback authentication **MUST** be used instead.

Evidence

The newly added conditional clauses permit all three headers to be absent without a secret, but
preceding text in the same parameter definitions retains unconditional MUST-level validation
requirements. The no-secret delivery mode therefore cannot satisfy all normative statements in the
operation.

ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[65-90]
ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[96-130]
ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[14-27]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The notification operation allows signature headers to be omitted for callback-authenticated subscriptions, while earlier normative text still tells receivers to perform signature-based validation for every notification.

## Fix Focus Areas
- ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[65-90]
- ovs_hub_ntf/v1/OVS_HUB_NTF_v1.0.0.yaml[96-130]

## Recommended Fix
Qualify the uniqueness, freshness, and HMAC-verification requirements so they apply only when a shared secret is configured and the signature headers are supplied; direct consumers using callback authentication to validate that mechanism instead.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[HenrikHL]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 24d970a

[Copilot]

Copilot review overview

🟡 Changes recommended

The component rename may break generated clients, and the authentication summary conflicts with the normative rules.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Clarify callback header optionality and shared-secret conditions · New

What changed in this PR

Updates OVS Hub notifications to support authenticated callbacks without requiring shared secrets or HMAC signatures, including secret lifecycle changes.

Changes:

• Makes authentication and signature headers conditional.
• Supports setting, replacing, and removing secrets.
• Updates schemas, examples, validation, and documentation.

File	Summary
ovs_hub_ntf/​v1/​OVS_HUB_NTF_v1.0.0.yaml	Updates authentication, subscription, secret management, schemas, and documentation.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.