Skip to content

GCP SM backend Get() doesn't return ErrNotFound when DB record exists but SM secret is missing #1441

Description

@ptone

Bug

The GCP Secret Manager backend's Get() method has asymmetric NotFound handling. When a DB record exists but the actual GCP SM secret resource is missing, the gRPC NotFound error is wrapped as a generic error instead of being converted to store.ErrNotFound. This silently breaks secret migration, which in turn breaks all chat integration plugins (Discord, Telegram, Slack) on any hub using the gcpsm backend where secrets weren't pre-provisioned in GCP SM.

Severity: High — Discord was down ~15 hours on scion-sagan before a manual workaround was applied.

Root cause

pkg/secret/gcpbackend.go:85-122 — the Get() method:

if s != nil {
    // Lines 98-103: accessLatestVersion returns gRPC NotFound
    // but it falls through to line 117-121:
    return nil, fmt.Errorf("failed to access secret value from GCP SM: %w", err)
    // ↑ wraps gRPC error, NOT store.ErrNotFound
} else {
    // Lines 104-116: correctly converts NotFound:
    if status.Code(err) == codes.NotFound {
        return nil, store.ErrNotFound  // ✅ correct
    }
}

The s != nil path (DB record exists, GCP SM resource missing) wraps the gRPC error generically. The s == nil path (no DB record) correctly converts to store.ErrNotFound.

Impact chain

  1. On initial hub setup, the DB secrets table gets stub records (key, scope, scope_id) with empty secret_ref and encrypted_value.
  2. MigratePluginSecrets (pkg/secretmigration/secretmigration.go:91) calls sb.Get() and checks: if err != nil && !errors.Is(err, store.ErrNotFound) { skip migration }.
  3. The s != nil path returns a wrapped gRPC error (not store.ErrNotFound), so migration is skipped.
  4. The secret never gets created in GCP SM.
  5. ResolvePluginConfig (pkg/config/integration_config.go:276) strips bot_token from inline config (by design — secret backend is authoritative).
  6. Plugin Configure() receives no bot_token → connection fails.

Fix

Add NotFound handling in the s != nil path of gcpbackend.go Get():

if s != nil {
    if smPath, ok := extractGCPSMPath(s.SecretRef); ok {
        value, err = b.accessLatestVersionByPath(ctx, smPath)
    } else {
        smName := b.gcpSecretName(name, scope, scopeID)
        value, err = b.accessLatestVersion(ctx, smName)
    }
    // ADD: treat GCP SM NotFound the same as no-DB-record NotFound
    if err != nil && status.Code(err) == codes.NotFound {
        return nil, store.ErrNotFound
    }
} else {
    // ... existing correct handling
}

Both the accessLatestVersion and accessLatestVersionByPath calls need the same treatment.

Reproduction

  1. Configure a plugin (discord/telegram) with bot_token in settings.yaml
  2. Enable gcpsm backend (SCION_SERVER_SECRETS_BACKEND=gcpsm)
  3. Ensure secrets table has a record but GCP SM does NOT have the secret resource
  4. Restart hub — plugin fails to connect

Files

  • pkg/secret/gcpbackend.go:85-122 — the asymmetric NotFound handling (the bug)
  • pkg/secretmigration/secretmigration.go:86-96 — migration caller that checks ErrNotFound
  • pkg/config/integration_config.go:276-287 — ResolvePluginConfig stripping bot_token
  • cmd/server_foreground.go:2664-2671 — initPluginManager calling ResolvePluginConfig

Reported by instance-investigator on scion-sagan (hub a2d6599). Manual workaround applied (gcloud secrets create from settings.yaml values).

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions