feat: Prefix machine account access tokens with bw_ behind feature flag

[maxkpower]

🎟️ Tracking

SM-2093, SM-2094

📔 Objective

Stacked on #8368. When the Sm2093MachineAccountTokenPrefix feature flag is enabled, CreateAccessTokenCommand prepends the literal bw_ to newly generated Secrets Manager machine account (service account) access token client secrets, before hashing. Behavior is unchanged when the flag is disabled.

This is backward compatible: token validation (SecretsManagerApiKeyProvider → Duende's stock HashedSharedSecretValidator) is a generic hash comparison with no assumption about secret shape, so already-issued unprefixed tokens continue to authenticate with no database or migration changes needed.

Adds test coverage for both flag states in CreateAccessTokenCommandTests.

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the bw_ client-secret prefix in CreateAccessTokenCommand behind Sm2093MachineAccountTokenPrefix, plus the unit test and the two new Api.IntegrationTest classes. The production change is a 5-line flag-gated concatenation before hashing: only ClientSecretHash is persisted (nvarchar(128), SHA-256 output length unchanged), SecretsManagerApiKeyProvider builds new Secret(apiKey.ClientSecretHash) for Duende's shape-agnostic hash comparison, and CreateAccessTokenCommand is the only writer of ClientSecretHash — so storage, validation, and previously issued tokens are unaffected on the server side. The random segment is still 30 alphanumeric characters from CoreHelpers.SecureRandomString, so the constant prefix costs no entropy, and injecting IFeatureService into a Commercial.Core SM command follows existing precedent (CreateSecretCommand, UpdateSecretCommand).

Two things worth noting outside the finding list: the Lint check is currently red, and the only net change this PR now makes to ServiceAccountsControllerTests.cs is incidental — the UTF-8 BOM was stripped from line 1 (.editorconfig sets charset = utf-8-bom for [*.{cs,csx,vb,vbx}]) and using Bit.Core; / using NSubstitute; were left behind after the flag-test classes were removed in 853c0ee; reverting that file to its base state would resolve both. The earlier question about SDK/CLI parsing of the composed 0.<id>.<secret>:<key> access token remains open in its thread and is worth confirming before the flag is enabled for anyone.

[claude]

❓ QUESTION: Have the token consumers been verified against the prefixed secret, not just server-side validation?

Why this is separate from the backward-compatibility argument

The PR description covers the inbound path correctly: SecretsManagerApiKeyProvider hands Duende a Secret(apiKey.ClientSecretHash) and the stock HashedSharedSecretValidator does a shape-agnostic hash comparison, so previously issued tokens keep working and the SHA-256 hash length is unchanged.

What that argument does not cover is the outbound path. This endpoint returns the bare ClientSecret (AccessTokenCreationResponseModel), and the user-facing access token is assembled outside this repo as 0.<accessTokenId>.<clientSecret>:<encryptionKey>, then parsed back apart by bws / the SDK before the client-credentials call is made. A parser that splits on . and : is unaffected, but one that constrains the secret segment by charset or length would reject bw_… — _ is outside the base64 alphabet, and the segment grows from 30 to 33 characters.

Worth confirming against the SDK/CLI access-token parser before the flag is turned on for anyone, since a failure there would surface as newly created machine accounts being unable to authenticate at all. If that has already been checked on the clients/SDK side, disregard.

[cd-bitwarden]

@maxkpower The change from 30 to 33 characters from adding the "bw_" prefix, does not impact value-based masking which is the approach used by nearly everyone.

The residual risk is limited to customer-defined regex patterns that explicitly assume a 30-character token. Those patterns could fail silently, but the likelihood and impact appear low enough that clearly documenting the change in the release notes is a proportionate mitigation.

• We should also update documentation to reflect this new formatting in the examples we show and a note about how users may see both variations as we are not changing legacy access tokens.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.49%. Comparing base (0c9f051) to head (e18e17d).

Additional details and impacted files

@@                            Coverage Diff                             @@
##           sm-2093-machine-account-token-prefix-flag    #8369   +/-   ##
==========================================================================
  Coverage                                      64.49%   64.49%           
==========================================================================
  Files                                           2480     2480           
  Lines                                         106110   106115    +5     
  Branches                                        9627     9628    +1     
==========================================================================
+ Hits                                           68435    68441    +6     
+ Misses                                         35337    35336    -1     
  Partials                                        2338     2338           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.