fix(security): backend validation for aws_external_id non-empty + length + charset on role_arn auth mode

[cristim]

Symptom

internal/api/handler_accounts.go:32 declares the External ID field with no validation:

type AccountRequest struct {
  ...
  AWSExternalID string `json:"aws_external_id"`
  ...
}

The frontend always populates it (see issue #18 / PR #36), but a buggy or hostile client can post aws_external_id: "". Combined with internal/credentials/resolver.go:165:

if account.AWSExternalID != "" {
  o.ExternalID = aws.String(account.AWSExternalID)
}

an empty stored External ID means STS AssumeRole is called without an ExternalId argument. Two outcomes:

• If the customer's trust policy has the sts:ExternalId condition: STS denies. Fails closed — fine.
• If the customer's trust policy lacks the condition: STS succeeds. The confused-deputy protection is silently lost.

The whole point of CUDly auto-generating the External ID is to remove the operator's ability to set it to a weak/empty value. Backend validation is the missing defence-in-depth layer.

Fix

In the AWS account create/update handler (internal/api/handler_accounts.go around line 32 / 335), reject auth_mode == "role_arn" with empty aws_external_id:

if req.AWSAuthMode == "role_arn" {
  if strings.TrimSpace(req.AWSExternalID) == "" {
    return NewClientError(400, "aws_external_id is required for role_arn auth mode")
  }
  if len(req.AWSExternalID) < 16 || len(req.AWSExternalID) > 1224 {
    return NewClientError(400, "aws_external_id must be 16-1224 characters (AWS sts:ExternalId limit)")
  }
  if !validExternalIDChars(req.AWSExternalID) {
    return NewClientError(400, "aws_external_id contains characters AWS sts:ExternalId does not accept (allowed: letters, digits, +=,.@:/-)")
  }
}

Where validExternalIDChars matches [\w+=,.@:/-] per AWS sts:ExternalId rules.

This validation should fire on both create and update paths.

Severity

Medium. Defence-in-depth — the frontend already always populates the field, so the immediate exposure depends on a hostile/buggy client. But the backend is the source of truth; no security check should be frontend-only.

Test coverage gap

No backend handler test exercises:

• Empty aws_external_id with auth_mode: role_arn is rejected.
• Out-of-range lengths are rejected.
• Disallowed characters are rejected.

Simple table-driven tests in handler_accounts_test.go would lock these in.

Related

• bug(onboarding): AWS External ID field empty and editable — should be auto-generated and read-only #18 — auto-generation in the frontend (closed by PR fix(onboarding): auto-generate AWS External ID and make field readonly #36)

[cristim]

PR #146 landed in fd5b348 — fix verified by tests in the PR. Recommending close.