Skip to content

test(auth): carve-out, recovery-code, deactivation and reset-token failures have no test #263

Closed

Description

@cristim

Summary

The internal/auth suite enumerates only the happy shape of several guards and pins the current behaviour where that behaviour is defective. The carve-out tables list the three exact (verb, purchases) pairs and never a wildcard resource, so the execute:* bypass (LeanerCloud/cloud-commitments-cli#1901) passes. The recovery-code test asserts only the in-memory slice with a succeeding UpdateUser, so the persist-failure defect (LeanerCloud/cloud-commitments-cli#1916) passes. The deactivation test registers no DeleteUserSessions expectation, so #89 passes and would keep passing after a fix unless the expectation is added. Nothing asserts that a deactivated account is refused a password reset, that an invited user cannot use forgot-password, that MFA setup on an already-enabled user (LeanerCloud/cloud-commitments-cli#1915) is refused, or that a signer whose first KMS call fails recovers on the second. TestResolveBastionProvider_LegacyFallback asserts NoError on the legacy credential fallback, pinning it as desired. Each linked fix needs its regression test, and this issue lists them in one place so none is forgotten.

Location

  • internal/auth/group_ceiling_permissions_test.go:24-28 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd
  • internal/auth/self_escalation_carveout_test.go:35-39
  • internal/auth/service_apikeys_test.go:1169-1188
  • internal/auth/service_mfa_test.go:438-466 (recovery code), :221, :236, :486 (MFA setup never with MFAEnabled=true)
  • internal/auth/service_user_test.go:646-665 (deactivation, no DeleteUserSessions expectation)
  • internal/auth/service_password_test.go:542-557 (reset token on inactive user treated as invite)
  • internal/auth/credentials/resolver_test.go:366-374 (legacy fallback pinned)
  • internal/oidc/aws_signer_test.go:29-34 (fake KMS never fails)

Failure scenario

A fix for any of the linked defects lands without its negative test, or a later refactor reintroduces the defect. The suite stays green because no test constructs the failing input: a grep of internal/auth/*_test.go for an execute permission with ResourceAll or "*" returns nothing, no test references PasswordSetupExpiry, and the fake KMS client's GetPublicKey never fails.

Evidence

carvedOut := []APIPermission{
    {Action: ActionExecute, Resource: ResourcePurchases},
    {Action: ActionApproveAny, Resource: ResourcePurchases},
    {Action: ActionRetryAny, Resource: ResourcePurchases},
}

Suggested fix


Found by the 2026-09-02 codebase audit, finding A03-025, reported by one reviewer and independently confirmed by a second. Full report: docs/audits/codebase-audit-2026-09-02.md.

Activity

  1. IAmJSD commented on Oct 19, 2026

    @IAmJSD
    Collaborator

    Claiming: working on this

  2. self-assigned this
    on Oct 19, 2026
  3. IAmJSD commented on Oct 28, 2026

    @IAmJSD
    Collaborator

    Acting-owner decision (D27, IAmJSD, 2026-10-28): merge #1556. Bastion accounts assume the target role with the bastion's own credentials, as the docs describe. A customer whose target role trusts the CUDly host instead must add the bastion as the trusted principal; this goes in the release notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions