Add CP outbound auth

[gabriel-farache]

When the Control-Plane (CP) has authN enabled, the agent will have to provide an Authorization token to be able to register and send its hearbeat.
3 options:

1. full: OIDP (keycloak) endpoint + client ID and client secret for the service account of the agent. Refresh is managed to ensure a valid token is always sent
2. static: only static Auth token, no refresh (best for dev and testing)
3. no auth

[gabriel-farache]

/agentic_review

[qodo-code-review]

Code Review by Qodo

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

1. Continuous integration cannot pass static checks 🐞 Bug ⚙ Maintainability

Description

The refresh test converts the integer expression '0'+seq directly to string, which Go vet's
stringintconv analyzer reports because integer-to-string conversion produces a rune rather than
decimal formatting. The repository's CI target runs go vet ./..., so the newly added test prevents
the required validation pipeline from succeeding.

Code

internal/dcm/token_test.go[101]

+					"access_token": "token-" + string('0'+seq),

Evidence

The changed expression is an integer-to-string conversion, and the repository explicitly runs Go vet
as part of its CI target.

internal/dcm/token_test.go[95-105]
Makefile[31-35]
Makefile[57-57]

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 token-refresh test converts an integer directly to `string`, which is rejected by Go vet's `stringintconv` analyzer.

Fix Focus Areas
- internal/dcm/token_test.go[95-105]

Recommended Fix
Construct the token value with explicit decimal formatting, such as `fmt.Sprintf("token-%d", seq)` or `strconv.FormatInt(int64(seq), 10)`, and add the required import.

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

---

2. Token outages leave capabilities stale 🐞 Bug ☼ Reliability

Description

setAuthHeader propagates token acquisition failures to reRegister, which logs the error and
returns without entering the registrar's retry loop. If the token endpoint is temporarily
unavailable during a service-type change, that notification is consumed and the old registration
remains active until another change or restart occurs.

Code

internal/dcm/client.go[R162-165]

+	token, err := c.tokenSource.Token(ctx)
+	if err != nil {
+		return fmt.Errorf("obtain auth token: %w", err)
+	}

Evidence

Token acquisition errors are returned before the registration HTTP request, while the lifecycle's
re-registration branch performs only one attempt and consumes notifications through a non-blocking
channel. The newly added requirement explicitly says failed authenticated requests are retried
through existing backoff behavior.

internal/dcm/client.go[158-169]
internal/dcm/registrar.go[214-222]
internal/dcm/registrar.go[306-319]
.ai/specs/environment-agent.spec.md[1114-1115]

Agent prompt

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

## Issue description
Authentication failures during re-registration are returned to `reRegister`, but that method logs once and drops the consumed service-type-change notification. A transient token endpoint outage can therefore leave the control plane with stale agent capabilities indefinitely.

## Fix Focus Areas
- internal/dcm/client.go[162-165]
- internal/dcm/registrar.go[306-319]

## Recommended Fix
Route re-registration failures through a context-aware retry loop using the existing registration backoff policy. Preserve non-retryable error handling, update the agent ID only after success, and add a test where token acquisition fails transiently and the same re-registration eventually succeeds without another notification.

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

---

3. Endpoint errors skip a validation layer 📘 Rule violation ≡ Correctness ⭐ New

Description

validateDCMAuth returns the error from validateAbsoluteHTTPURL directly instead of wrapping it
with %w. When token-endpoint URL validation fails, the error chain jumps from general
configuration validation to the URL helper without identifying the client-credentials validation
boundary.

Code

internal/config/config.go[R290-291]

+	if err := validateAbsoluteHTTPURL("DCM_AUTH_TOKEN_ENDPOINT", endpoint); err != nil {
+		return err

Evidence

Compliance rule 2788501 requires functions returning lower-layer errors to add context with
fmt.Errorf and %w. The newly added validateDCMAuth branch returns the URL validator's error
unchanged.

Rule 2788501: Wrap errors with context using fmt.Errorf and %w
internal/config/config.go[290-291]

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 DCM authentication validator returns token-endpoint URL errors without adding context for its own validation boundary.

## Fix Focus Areas
- internal/config/config.go[290-291]

## Recommended Fix
Replace the bare return with `fmt.Errorf` containing client-credentials token-endpoint validation context and wrap the original error using `%w`.

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

---

4. Auth errors omit the request stage 📘 Rule violation ≡ Correctness ⭐ New

Description

register and heartbeat return errors from setAuthHeader directly rather than wrapping them
with request-specific context. When token acquisition fails, the propagated error identifies
authentication but not which outbound operation was being prepared, obscuring the failure path for
callers and logs.

Code

internal/dcm/client.go[R84-85]

+	if err := c.setAuthHeader(ctx, req); err != nil {
+		return "", err

Evidence

Compliance rule 2788501 requires functions to wrap returned errors with contextual fmt.Errorf
messages and %w. Both newly added authentication branches return the helper error unchanged.

Rule 2788501: Wrap errors with context using fmt.Errorf and %w
internal/dcm/client.go[84-85]
internal/dcm/client.go[142-143]

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 registration and heartbeat methods return authentication errors directly, omitting the outbound operation that failed.

## Fix Focus Areas
- internal/dcm/client.go[84-85]
- internal/dcm/client.go[142-143]

## Recommended Fix
Wrap each `setAuthHeader` error with `fmt.Errorf` and `%w`, using distinct registration and heartbeat context while preserving the underlying error chain.

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

---

5. Malformed expiry disables token caching 🐞 Bug ☼ Reliability ⭐ New

Description

fetchToken converts expires_in directly to time.Duration without rejecting missing,
non-positive, or overflowing values. Such a response stores an expiry at or before the current time,
so every subsequent registration or heartbeat fetches another token instead of reusing the cached
one.

Code

internal/dcm/token.go[R125-126]

+	expiry := time.Now().Add(time.Duration(tokenResp.ExpiresIn) * time.Second)
+	return tokenResp.AccessToken, expiry, nil

Evidence

The response model accepts any int64, and line 125 multiplies it without range or sign validation.
Cache reuse at lines 76-78 requires the resulting expiry to be more than the safety delta in the
future, while registration and heartbeat both request a token through the new authorization path.

internal/dcm/token.go[72-86]
internal/dcm/token.go[89-92]
internal/dcm/token.go[121-126]
internal/dcm/client.go[158-168]

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 token response accepts missing, non-positive, and overflowing `expires_in` values, which can make the cached token immediately stale and force a new token request for every outbound control-plane request.

## Fix Focus Areas
- internal/dcm/token.go[89-92]
- internal/dcm/token.go[121-126]

## Recommended Fix
Validate that `expires_in` is positive and can be converted to `time.Duration` without overflow before computing the expiry. Return a descriptive token-response error when validation fails, and add tests for zero, negative, and overflowing values.

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

---

View medium (5)

6. Service updates can stall for a minute 🐞 Bug ☼ Reliability ⭐ New

Description

reRegister now delegates to registerWithRetry with the long-lived registrar context, so each DCM
request is limited only by the client's 60-second timeout rather than its former 10-second request
context. When DCM accepts a re-registration connection but does not reply, the single event loop
cannot apply service-type updates or process heartbeats for up to a minute before backoff begins.

Code

internal/dcm/registrar.go[331]

+	r.registerWithRetry(ctx, "re-registered with DCM")

Evidence

The changed call passes the registrar context into the shared retry helper. That helper passes it
unchanged to client.register, whose request is therefore governed by the 60-second DCM HTTP client
timeout; because reRegister runs synchronously in the select loop, heartbeat and notification
handling cannot resume until that request returns.

internal/dcm/registrar.go[214-223]
internal/dcm/registrar.go[242-283]
internal/dcm/registrar.go[321-331]
internal/dcm/client.go[64-90]

Agent prompt

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

### Issue description
Re-registration previously bounded each DCM registration attempt with a 10-second child context. Routing it through `registerWithRetry` passes the long-lived registrar context to the request instead, extending a hung DCM attempt to the client's 60-second timeout and blocking the registrar event loop.

### Fix Focus Areas
- internal/dcm/registrar.go[321-331]

### Recommended Fix
Preserve a 10-second deadline for each re-registration attempt while retaining the shared retry loop. Add a per-attempt timeout mechanism to the retry helper, or a re-registration-specific attempt callback, so every retry creates a fresh child context derived from the parent; do not wrap the entire retry loop in one 10-second context.

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

---

7. Token failures churn network connections 🐞 Bug ➹ Performance ⭐ New

Description

fetchToken returns immediately for non-200 responses without consuming the response body before
its deferred close. Repeated authentication failures therefore prevent the HTTP transport from
reusing those connections, adding a new connection and TLS handshake to successive registration
retries.

Code

internal/dcm/token.go[R113-115]

+	if resp.StatusCode != http.StatusOK {
+		return "", time.Time{}, fmt.Errorf("token endpoint returned HTTP %d", resp.StatusCode)
+	}

Evidence

The body is scheduled for close at line 111, but the non-200 branch returns at lines 113-115 before
reading it. Token acquisition is repeatedly entered through Token, including the registrar's retry
path after acquisition errors.

internal/dcm/token.go[72-86]
internal/dcm/token.go[107-115]
internal/dcm/registrar.go[242-282]

Agent prompt

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

## Issue description
Non-200 token responses are closed without being consumed, preventing HTTP keep-alive connection reuse across repeated authentication failures.

## Fix Focus Areas
- internal/dcm/token.go[107-115]

## Recommended Fix
Before returning for a non-200 status, read and discard a bounded amount of the response body and then close it. Keep the bound small, and optionally include a sanitized bounded response excerpt in the returned error for diagnostics.

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

---

8. Startup errors lose validation context 📘 Rule violation ≡ Correctness

Description

Validate returns the error from validateDCMAuth directly instead of wrapping it with
fmt.Errorf and %w. Partial client-credential configuration takes this branch during startup, so
the top-level configuration failure carries no indication that it passed through the
outbound-authentication validation step.

Code

internal/config/config.go[R258-259]

+	if err := c.validateDCMAuth(); err != nil {
+		return err

Evidence

Compliance rule 2788501 requires contextual %w wrapping rather than bare error returns; the newly
added authentication-validation branch returns err directly.

Rule 2788501: Wrap errors with context using fmt.Errorf and %w
internal/config/config.go[258-259]

Agent prompt

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

## Issue description
`Config.Validate` propagates the outbound-authentication validation error without adding context or preserving it through an explicit `%w` wrapper.

## Fix Focus Areas
- internal/config/config.go[258-259]

## Recommended Fix
Return `fmt.Errorf("validating DCM authentication: %w", err)` from this branch so the validation layer adds context while preserving the underlying error.

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

---

9. Bad token addresses retry forever 🐞 Bug ☼ Reliability

Description

validateDCMAuth checks only whether all three client-credentials values are present and never
validates that AuthTokenEndpoint is an absolute HTTP endpoint. When the configured value is
malformed or uses an unsupported scheme, fetchToken fails before reaching a server and
doRegistration repeatedly backs off instead of rejecting the configuration at startup.

Code

internal/config/config.go[R267-270]

+func (c *Config) validateDCMAuth() error {
+	endpoint := c.DCM.AuthTokenEndpoint
+	clientID := c.DCM.AuthClientID
+	clientSecret := c.DCM.AuthClientSecret

Evidence

Startup invokes Config.Validate, but the new auth validator only checks presence. The unvalidated
string is stored directly in the token source, where request creation or transport rejects invalid
addresses; initial registration treats those errors as retryable and loops with backoff.

cmd/environment-agent/main.go[77-85]
internal/config/config.go[264-290]
internal/dcm/token.go[63-69]
internal/dcm/token.go[101-109]
internal/dcm/registrar.go[226-268]

Agent prompt

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

## Issue description
A fully populated client-credentials configuration passes startup validation even when its token endpoint is malformed, relative, hostless, or uses an unsupported scheme. Token acquisition then fails on every registration attempt and the registrar retries a permanent local configuration error indefinitely.

## Fix Focus Areas
- internal/config/config.go[267-290]
- internal/dcm/token.go[101-109]

## Recommended Fix
Parse `DCM_AUTH_TOKEN_ENDPOINT` during configuration validation and require an absolute URL with a nonempty host and an allowed HTTP or HTTPS scheme. Return a startup validation error naming the environment variable, and add tests for malformed, relative, hostless, and unsupported-scheme values.

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

---

10. Token failures lose source context 📘 Rule violation ≡ Correctness

Description

Token returns the error from fetchToken directly instead of wrapping it with fmt.Errorf and
%w. When request creation, transport, or response decoding fails, every registration and heartbeat
path receives only the lower-layer message without the token-acquisition boundary.

Code

internal/dcm/token.go[R80-82]

+	token, expiry, err := c.fetchToken(ctx)
+	if err != nil {
+		return "", err

Evidence

Compliance rule 2788501 requires functions to wrap returned errors with contextual fmt.Errorf
messages using %w; the new Token implementation returns err unchanged.

Rule 2788501: Wrap errors with context using fmt.Errorf and %w
internal/dcm/token.go[80-82]

Agent prompt

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

## Issue description
`ClientCredentialsTokenSource.Token` returns the error from `fetchToken` without adding call-site context, contrary to the error-wrapping requirement.

## Fix Focus Areas
- internal/dcm/token.go[80-82]

## Recommended Fix
Replace the bare return with `fmt.Errorf("fetching client-credentials token: %w", err)` so callers retain both contextual information and the wrapped cause.

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

---

11. Token state lacks category headers ✗ Dismissed 📘 Rule violation ⚙ Maintainability

Description

ClientCredentialsTokenSource mixes endpoint credentials, HTTP infrastructure, refresh settings,
and mutable cache state without category comments. A later change to refresh or transport behavior
has only whitespace to distinguish ownership, making field placement and dependency classification
ambiguous.

Code

internal/dcm/token.go[R44-47]

+type ClientCredentialsTokenSource struct {
+	tokenEndpoint string
+	clientID      string
+	clientSecret  string

Evidence

Compliance rule 2788534 requires struct fields to be separated into logical categories with comment
headers; this new struct uses only an uncommented blank line between its
configuration/infrastructure fields and cached state.

Rule 2788534: Struct fields should be grouped by category with comments
internal/dcm/token.go[44-54]

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 new client-credentials token source contains configuration, infrastructure, and mutable cache fields but does not identify those logical categories with comment headers.

## Fix Focus Areas
- internal/dcm/token.go[44-54]

## Recommended Fix
Reorder the fields into logical groups and add concise headers such as `// config`, `// infra`, and `// cached state`, keeping the mutex and protected token state together.

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

---

Context sources

✅ Compliance rules (platform): 17 rules

Review mode: 🧠 Deep: This push adds security-sensitive outbound authentication, token caching/refresh, configuration validation, and retry behavior across multiple independent runtime paths, creating substantial defect density that benefits from redundant review passes.

Tip of the day

💡 Did you know, you can turn these tips off under Display preferences

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 4744890

Results up to commit 2d1fe1a 🧠 Deep

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

1. Token outages leave capabilities stale 🐞 Bug ☼ Reliability

Description

setAuthHeader propagates token acquisition failures to reRegister, which logs the error and
returns without entering the registrar's retry loop. If the token endpoint is temporarily
unavailable during a service-type change, that notification is consumed and the old registration
remains active until another change or restart occurs.

Code

internal/dcm/client.go[R162-165]

+	token, err := c.tokenSource.Token(ctx)
+	if err != nil {
+		return fmt.Errorf("obtain auth token: %w", err)
+	}

Evidence

Token acquisition errors are returned before the registration HTTP request, while the lifecycle's
re-registration branch performs only one attempt and consumes notifications through a non-blocking
channel. The newly added requirement explicitly says failed authenticated requests are retried
through existing backoff behavior.

internal/dcm/client.go[158-169]
internal/dcm/registrar.go[214-222]
internal/dcm/registrar.go[306-319]
.ai/specs/environment-agent.spec.md[1114-1115]

Agent prompt

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

## Issue description
Authentication failures during re-registration are returned to `reRegister`, but that method logs once and drops the consumed service-type-change notification. A transient token endpoint outage can therefore leave the control plane with stale agent capabilities indefinitely.

## Fix Focus Areas
- internal/dcm/client.go[162-165]
- internal/dcm/registrar.go[306-319]

## Recommended Fix
Route re-registration failures through a context-aware retry loop using the existing registration backoff policy. Preserve non-retryable error handling, update the agent ID only after success, and add a test where token acquisition fails transiently and the same re-registration eventually succeeds without another notification.

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

---

2. Continuous integration cannot pass static checks 🐞 Bug ⚙ Maintainability

Description

The refresh test converts the integer expression '0'+seq directly to string, which Go vet's
stringintconv analyzer reports because integer-to-string conversion produces a rune rather than
decimal formatting. The repository's CI target runs go vet ./..., so the newly added test prevents
the required validation pipeline from succeeding.

Code

internal/dcm/token_test.go[101]

+					"access_token": "token-" + string('0'+seq),

Evidence

The changed expression is an integer-to-string conversion, and the repository explicitly runs Go vet
as part of its CI target.

internal/dcm/token_test.go[95-105]
Makefile[31-35]
Makefile[57-57]

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 token-refresh test converts an integer directly to `string`, which is rejected by Go vet's `stringintconv` analyzer.

Fix Focus Areas
- internal/dcm/token_test.go[95-105]

Recommended Fix
Construct the token value with explicit decimal formatting, such as `fmt.Sprintf("token-%d", seq)` or `strconv.FormatInt(int64(seq), 10)`, and add the required import.

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

---

3. Token failures lose source context 📘 Rule violation ≡ Correctness

Description

Token returns the error from fetchToken directly instead of wrapping it with fmt.Errorf and
%w. When request creation, transport, or response decoding fails, every registration and heartbeat
path receives only the lower-layer message without the token-acquisition boundary.

Code

internal/dcm/token.go[R80-82]

+	token, expiry, err := c.fetchToken(ctx)
+	if err != nil {
+		return "", err

Evidence

Compliance rule 2788501 requires functions to wrap returned errors with contextual fmt.Errorf
messages using %w; the new Token implementation returns err unchanged.

Rule 2788501: Wrap errors with context using fmt.Errorf and %w
internal/dcm/token.go[80-82]

Agent prompt

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

## Issue description
`ClientCredentialsTokenSource.Token` returns the error from `fetchToken` without adding call-site context, contrary to the error-wrapping requirement.

## Fix Focus Areas
- internal/dcm/token.go[80-82]

## Recommended Fix
Replace the bare return with `fmt.Errorf("fetching client-credentials token: %w", err)` so callers retain both contextual information and the wrapped cause.

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

---

4. Startup errors lose validation context 📘 Rule violation ≡ Correctness

Description

Validate returns the error from validateDCMAuth directly instead of wrapping it with
fmt.Errorf and %w. Partial client-credential configuration takes this branch during startup, so
the top-level configuration failure carries no indication that it passed through the
outbound-authentication validation step.

Code

internal/config/config.go[R258-259]

+	if err := c.validateDCMAuth(); err != nil {
+		return err

Evidence

Compliance rule 2788501 requires contextual %w wrapping rather than bare error returns; the newly
added authentication-validation branch returns err directly.

Rule 2788501: Wrap errors with context using fmt.Errorf and %w
internal/config/config.go[258-259]

Agent prompt

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

## Issue description
`Config.Validate` propagates the outbound-authentication validation error without adding context or preserving it through an explicit `%w` wrapper.

## Fix Focus Areas
- internal/config/config.go[258-259]

## Recommended Fix
Return `fmt.Errorf("validating DCM authentication: %w", err)` from this branch so the validation layer adds context while preserving the underlying error.

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

---

5. Bad token addresses retry forever 🐞 Bug ☼ Reliability

Description

validateDCMAuth checks only whether all three client-credentials values are present and never
validates that AuthTokenEndpoint is an absolute HTTP endpoint. When the configured value is
malformed or uses an unsupported scheme, fetchToken fails before reaching a server and
doRegistration repeatedly backs off instead of rejecting the configuration at startup.

Code

internal/config/config.go[R267-270]

+func (c *Config) validateDCMAuth() error {
+	endpoint := c.DCM.AuthTokenEndpoint
+	clientID := c.DCM.AuthClientID
+	clientSecret := c.DCM.AuthClientSecret

Evidence

Startup invokes Config.Validate, but the new auth validator only checks presence. The unvalidated
string is stored directly in the token source, where request creation or transport rejects invalid
addresses; initial registration treats those errors as retryable and loops with backoff.

cmd/environment-agent/main.go[77-85]
internal/config/config.go[264-290]
internal/dcm/token.go[63-69]
internal/dcm/token.go[101-109]
internal/dcm/registrar.go[226-268]

Agent prompt

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

## Issue description
A fully populated client-credentials configuration passes startup validation even when its token endpoint is malformed, relative, hostless, or uses an unsupported scheme. Token acquisition then fails on every registration attempt and the registrar retries a permanent local configuration error indefinitely.

## Fix Focus Areas
- internal/config/config.go[267-290]
- internal/dcm/token.go[101-109]

## Recommended Fix
Parse `DCM_AUTH_TOKEN_ENDPOINT` during configuration validation and require an absolute URL with a nonempty host and an allowed HTTP or HTTPS scheme. Return a startup validation error naming the environment variable, and add tests for malformed, relative, hostless, and unsupported-scheme values.

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

---

6. Token state lacks category headers ✗ Dismissed 📘 Rule violation ⚙ Maintainability

Description

ClientCredentialsTokenSource mixes endpoint credentials, HTTP infrastructure, refresh settings,
and mutable cache state without category comments. A later change to refresh or transport behavior
has only whitespace to distinguish ownership, making field placement and dependency classification
ambiguous.

Code

internal/dcm/token.go[R44-47]

+type ClientCredentialsTokenSource struct {
+	tokenEndpoint string
+	clientID      string
+	clientSecret  string

Evidence

Compliance rule 2788534 requires struct fields to be separated into logical categories with comment
headers; this new struct uses only an uncommented blank line between its
configuration/infrastructure fields and cached state.

Rule 2788534: Struct fields should be grouped by category with comments
internal/dcm/token.go[44-54]

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 new client-credentials token source contains configuration, infrastructure, and mutable cache fields but does not identify those logical categories with comment headers.

## Fix Focus Areas
- internal/dcm/token.go[44-54]

## Recommended Fix
Reorder the fields into logical groups and add concise headers such as `// config`, `// infra`, and `// cached state`, keeping the mutex and protected token state together.

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

---

[gabriel-farache]

@chadcrum PTAL on this one too for authenticating to the CP from the agent

[qodo-code-review]

PR Summary by Qodo

Add outbound control-plane authentication and resilient re-registration

✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Authenticate control-plane registration and heartbeats using static or OAuth2 client-credentials
 tokens.
• Cache and proactively refresh OAuth2 tokens while preserving unauthenticated backward
 compatibility.
• Retry failed re-registrations and expand configuration, documentation, and automated coverage.

Diagram

sequenceDiagram
    participant Config as Auth Config
    participant Registrar as Registrar
    participant Tokens as Token Source
    participant OIDC as OIDC Endpoint
    participant Client as DCM Client
    participant CP as Control Plane
    Config->>Registrar: Resolve auth mode
    Registrar->>Client: Register or heartbeat
    alt Client credentials
        Client->>Tokens: Request token
        opt Token near expiry
            Tokens->>OIDC: Fetch access token
            OIDC-->>Tokens: Return JWT
        end
        Tokens-->>Client: Cached JWT
    else Static token
        Client->>Tokens: Request token
        Tokens-->>Client: Fixed JWT
    else No authentication
        Note over Client: Omit Authorization
    end
    Client->>CP: Send request
    CP-->>Client: Return response
    Registrar->>Registrar: Retry registration failures

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use golang.org/x/oauth2 client credentials

• ➕ Uses a widely reviewed OAuth2 implementation
• ➕ Handles more provider authentication conventions and protocol edge cases
• ➕ Reduces long-term maintenance of security-sensitive code
• ➖ Promotes an existing indirect dependency to a direct dependency
• ➖ Adds abstraction for a narrowly scoped grant flow
• ➖ May require adapters to preserve current error and cache behavior

Recommendation: Consider using golang.org/x/oauth2/clientcredentials because token acquisition is security-sensitive and OIDC providers may differ in client-auth conventions. The current TokenSource abstraction should remain either way; the hand-rolled implementation is acceptable only if the supported endpoint contract is intentionally narrow and its response and authentication semantics are documented.

Files changed (15) +1240 / -39

Enhancement (3) +175 / -5

main.goResolve and inject the configured token source +19/-0

Resolve and inject the configured token source

• Builds the outbound TokenSource during startup, preferring client credentials over a static token. Passes nil when authentication is not configured to preserve existing behavior.

cmd/environment-agent/main.go

client.goAttach bearer tokens to outbound DCM requests +29/-5

Attach bearer tokens to outbound DCM requests

• Adds TokenSource support to the DCM client and resolves a token before registration and heartbeat requests. Token acquisition errors are propagated without sending the request, while nil sources omit authorization.

internal/dcm/client.go

token.goImplement static and client-credentials token sources +127/-0

Implement static and client-credentials token sources

• Introduces a concurrency-safe TokenSource abstraction with fixed-token and OAuth2 client-credentials implementations. Client-credentials tokens are cached and refreshed ten seconds before expiry, with endpoint failures returned to callers.

internal/dcm/token.go

Bug fix (1) +28 / -15

registrar.goInject authentication and share registration retry handling +28/-15

Inject authentication and share registration retry handling

• Passes the configured TokenSource into the DCM client. Extracts a shared retry loop so re-registration survives transient failures and only replaces the agent ID after success.

internal/dcm/registrar.go

Tests (4) +550 / -17

config_test.goTest DCM authentication configuration handling +101/-0

Test DCM authentication configuration handling

• Covers environment parsing, missing client-credentials fields, unauthenticated defaults, and valid or invalid token endpoint URLs.

internal/config/config_test.go

registrar_integration_test.goVerify authenticated requests and re-registration recovery +275/-12

Verify authenticated requests and re-registration recovery

• Captures Authorization headers and tests static tokens, client credentials, refresh, precedence, and no-auth requests. Adds a flaky token source scenario proving re-registration resumes after token service recovery without another notification.

internal/dcm/registrar_integration_test.go

token_test.goTest token fetching, caching, refresh, and concurrency +169/-0

Test token fetching, caching, refresh, and concurrency

• Verifies static-token behavior and client-credentials request formation, caching, proactive refresh, error propagation, and concurrent access safety.

internal/dcm/token_test.go

routing_integration_test.goMake routing retry test deterministic +5/-5

Make routing retry test deterministic

• Replaces a timing-dependent goroutine race with the fake forwarder's FailFirst behavior, ensuring exactly the first create attempt fails before retry succeeds.

internal/routing/routing_integration_test.go

Documentation (3) +254 / -2

environment-agent.decisions.mdDocument outbound authentication and shared registration retries +81/-0

Document outbound authentication and shared registration retries

• Records the three authentication modes, precedence, token refresh strategy, and rationale for a custom TokenSource. Also documents the decision to share retry behavior between initial registration and re-registration, including heartbeat trade-offs.

.ai/decisions/environment-agent.decisions.md

environment-agent.spec.mdSpecify CP authentication and resilient re-registration requirements +104/-2

Specify CP authentication and resilient re-registration requirements

• Adds requirements, configuration keys, and acceptance criteria for static tokens, client credentials, refresh, precedence, no-auth compatibility, and partial-config validation. Adds a requirement ensuring transient re-registration failures are retried without another notification.

.ai/specs/environment-agent.spec.md

README.mdDocument control-plane authentication setup +69/-0

Document control-plane authentication setup

• Explains client-credentials, static-token, and no-auth modes, including precedence and refresh behavior. Provides Keycloak and Kubernetes Secret setup guidance and describes static-token limitations.

README.md

Other (4) +233 / -0

2026-07-17-16-28-integration-tests.mdPlan end-to-end authentication and re-registration coverage +67/-0

Plan end-to-end authentication and re-registration coverage

• Defines integration scenarios for authenticated registration and heartbeat, token refresh, no-auth behavior, and recovery from transient token outages. Updates acceptance-criteria traceability mappings.

.ai/test-plans/2026-07-17-16-28-integration-tests.md

2026-07-17-16-28-unit-tests.mdPlan unit coverage for auth configuration and token sources +107/-0

Plan unit coverage for auth configuration and token sources

• Adds unit scenarios for environment parsing, partial configuration validation, static tokens, client-credentials fetching, caching, refresh, failures, and concurrency. Extends acceptance-criteria traceability.

.ai/test-plans/2026-07-17-16-28-unit-tests.md

config.goAdd and validate DCM authentication configuration +39/-0

Add and validate DCM authentication configuration

• Adds static-token and OAuth2 client-credentials fields to DCM configuration. Rejects partial client-credentials settings and validates the configured token endpoint during startup.

internal/config/config.go

validation.goValidate absolute HTTP endpoint URLs +20/-0

Validate absolute HTTP endpoint URLs

• Introduces reusable validation requiring endpoint URLs to be absolute, host-qualified, and use HTTP or HTTPS.

internal/config/validation.go

[qodo-code-review]

Code review by qodo was updated up to the latest commit 705df0d