diff --git a/AGENTS.md b/AGENTS.md index f6b13c3..c070d4f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -281,8 +281,7 @@ For full flag/argument reference, use `band --help`. This section cove - **`sip realm create --if-not-exists` does not always silently reuse.** If a realm with that name exists but its `default` or `description` differs from what was requested, the command exits **4** instead of reusing it. Reconcile with `sip realm update --description ` (or promote it with `--default=true`) - rather than retrying create. Realm names match case-insensitively, so `--name VAPI` reuses an existing - `vapi`. + rather than retrying create. Realm names are validated to lowercase alphanumeric only (`[a-z0-9]`) — uppercase and hyphens are rejected before any API call. - **`sip realm create --if-not-exists --wait` is safe to combine.** A reused realm that is still `CREATE_PENDING` is polled to `ACTIVE` before the command returns, so a re-run after a `--wait` timeout cannot hand back exit 0 with a realm that is not yet usable. @@ -986,24 +985,37 @@ FROM=$(band number list --plain | jq -r '.[0]') # On Bandwidth Build accounts, band number list is not available. # Pass the pre-provisioned number manually: FROM=+19195551234 -# 2. Create an ephemeral realm (never the default — it cannot be deleted if it is) -REALM=$(band sip realm create --name sip-test --default=false --wait --plain) +# 2. Create an ephemeral realm (never the default — it cannot be deleted if it is). +# Realm names must be lowercase alphanumeric only (a-z, 0-9) — no hyphens. +REALM=$(band sip realm create --name siptest --default=false --wait --plain) REALM_ID=$(echo "$REALM" | jq -r '.id') REALM_FQDN=$(echo "$REALM" | jq -r '.hostname') +# DNS for a new realm FQDN can take 1–2 minutes to propagate after ACTIVE status. +# Poll until it resolves before proceeding — dialing an unresolvable FQDN fails +# immediately with no useful error from the SIP UA. +until dig +short "$REALM_FQDN" | grep -q '.'; do + echo "Waiting for DNS ($REALM_FQDN)..."; sleep 10 +done + # 3. Create a credential and capture the password — printed exactly once CRED=$(band sip credential create \ --realm "$REALM_ID" \ - --username sip-test-agent \ + --username siptestagent \ --generate-password \ --plain) CRED_ID=$(echo "$CRED" | jq -r '.id') SIP_PASS=$(echo "$CRED" | jq -r '.password') +# Bandwidth's SIP proxy caches credential hashes. Allow ~15 s for the new +# credential to propagate before dialing — otherwise auth challenges fail +# even though the digest response is mathematically correct. +sleep 15 + # 4. Write a temp accounts file — 600 permissions, never passed on the command line TMPDIR=$(mktemp -d) chmod 700 "$TMPDIR" -printf ';regint=0;audio_codecs=pcmu/8000,pcma/8000;auth_user=sip-test-agent;auth_pass=%s\n' \ +printf ';regint=0;audio_codecs=pcmu/8000,pcma/8000;auth_user=siptestagent;auth_pass=%s\n' \ "$FROM" "$REALM_FQDN" "$SIP_PASS" > "$TMPDIR/accounts" chmod 600 "$TMPDIR/accounts" unset SIP_PASS # clear from environment immediately after writing @@ -1027,11 +1039,12 @@ band sip realm delete "$REALM_ID" --wait | Signal | Meaning | |--------|---------| -| `407 Proxy Authentication Required` → re-INVITE | Auth challenge is working. Wait for the response to the re-INVITE. | +| `407 Proxy Authentication Required` → re-INVITE → `180`/`183` | Auth challenge accepted. Call is connecting. | +| `407` repeating after re-INVITE (no `180`/`183`) | SIP proxy credential cache hasn't refreshed. Wait 15 s and retry from step 5. | | `180 Ringing` or `183 Session Progress` | Call reached the PSTN. | | `200 OK` + "Call established" | Call connected. SIP auth is fully functional. | | `403 Forbidden` after the re-INVITE | Credential mismatch — the password written to the accounts file does not match the stored hashes. Rotate the credential and retry from step 3. | -| `503 Service Unavailable` or `480` | Routing issue. Check that the realm is `ACTIVE` (`band sip realm get sip-test --plain`) and that the FQDN has propagated in DNS — new realms may take a moment. | +| `503 Service Unavailable` or `480` | Routing issue. Check that the realm is `ACTIVE` (`band sip realm get siptest --plain`) and that the FQDN has propagated in DNS — new realms can take 1–2 minutes. | **Always clean up in step 6**, even on failure. A leftover credential is not a security risk (Bandwidth never stores or returns the plaintext password), but unused credentials and realms should not accumulate. diff --git a/cmd/sip/realm_create_test.go b/cmd/sip/realm_create_test.go index 6e0930d..e713607 100644 --- a/cmd/sip/realm_create_test.go +++ b/cmd/sip/realm_create_test.go @@ -81,44 +81,28 @@ func TestRealmCreate_IfNotExistsWithWaitPollsPendingRealmToActive(t *testing.T) } } -// TestRealmCreate_IfNotExistsMatchesNameCaseInsensitively covers spec line 29: -// realm names compare case-insensitively. ValidateRealmName accepts uppercase, -// but the name is a DNS label the API may normalize to lowercase — so an exact -// comparison makes `--name VAPI --if-not-exists` create a second realm (or -// fail) every single run instead of reusing the existing `vapi`. -func TestRealmCreate_IfNotExistsMatchesNameCaseInsensitively(t *testing.T) { - var posts int32 +// TestRealmCreate_IfNotExistsRejectsUppercaseName verifies that uppercase realm +// names are rejected before any API call. The API only accepts [a-z0-9] (error +// 33013), so validation must catch this early — an uppercase name that reaches +// the server would fail unpredictably and potentially burn a generated password. +func TestRealmCreate_IfNotExistsRejectsUppercaseName(t *testing.T) { + var requests int32 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodPost { - atomic.AddInt32(&posts, 1) - } - if strings.HasSuffix(r.URL.Path, "/realms") && r.Method == http.MethodGet { - w.Write([]byte(realmListXML("vapi", "ACTIVE", false))) - return - } - w.WriteHeader(404) + atomic.AddInt32(&requests, 1) + w.WriteHeader(500) })) defer srv.Close() withStubService(t, srv) root := testutil.NewTestRoot(realmCreateCmd) - root.SetArgs([]string{"create", "--name", "VAPI", "--default=false", "--if-not-exists", "--wait=false", "--plain"}) - - out := testutil.CaptureStdout(t, func() { - if err := root.Execute(); err != nil { - t.Fatalf("Execute() error = %v", err) - } - }) + root.SetArgs([]string{"create", "--name", "VAPI", "--default=false", "--if-not-exists", "--plain"}) - if n := atomic.LoadInt32(&posts); n != 0 { - t.Errorf("issued %d POSTs — a mixed-case --name must match the existing lowercase realm, not create a new one", n) - } - var got map[string]interface{} - if err := json.Unmarshal(bytes.TrimSpace([]byte(out)), &got); err != nil { - t.Fatalf("not JSON: %q (%v)", out, err) + err := root.Execute() + if err == nil { + t.Fatal("Execute() = nil, want error for uppercase realm name") } - if got["name"] != "vapi" { - t.Errorf("name = %v, want the existing realm vapi", got["name"]) + if n := atomic.LoadInt32(&requests); n != 0 { + t.Errorf("issued %d HTTP requests — validation must reject uppercase names before any API call", n) } } diff --git a/internal/sip/sip.go b/internal/sip/sip.go index 7de1edd..5f5a569 100644 --- a/internal/sip/sip.go +++ b/internal/sip/sip.go @@ -18,9 +18,10 @@ const passwordAlphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz01 const passwordLength = 22 -// realmNameRe enforces a DNS label. The two alternatives exist because a single -// pattern permitting a trailing hyphen would emit an invalid hostname label. -var realmNameRe = regexp.MustCompile(`^([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9-]{0,28}[A-Za-z0-9])$`) +// realmNameRe enforces the Bandwidth API constraint for realm names (error 33013): +// lowercase alphanumeric only. This is stricter than a DNS label — hyphens and +// uppercase letters are rejected by the API even though DNS labels permit them. +var realmNameRe = regexp.MustCompile(`^[a-z0-9]+$`) // ComputeHashes returns the SIP digest hashes Bandwidth requires when creating // or rotating a credential. Bandwidth does not compute these server-side. @@ -67,8 +68,8 @@ func GeneratePassword() (string, error) { return b.String(), nil } -// ValidateRealmName checks that name is a usable DNS label. The realm name -// becomes the first label of the realm's FQDN. +// ValidateRealmName checks that name satisfies the Bandwidth API constraint. +// The name becomes the first label of the realm's FQDN. func ValidateRealmName(name string) error { if name == "" { return fmt.Errorf("realm name is required") @@ -77,7 +78,7 @@ func ValidateRealmName(name string) error { return fmt.Errorf("realm name %q is %d characters; maximum is 30", name, len(name)) } if !realmNameRe.MatchString(name) { - return fmt.Errorf("realm name %q must be alphanumeric with internal hyphens only (a-z, A-Z, 0-9, -)", name) + return fmt.Errorf("realm name %q must be lowercase alphanumeric only (a-z, 0-9)", name) } return nil } diff --git a/internal/sip/sip_test.go b/internal/sip/sip_test.go index 3a1b371..455cf52 100644 --- a/internal/sip/sip_test.go +++ b/internal/sip/sip_test.go @@ -50,14 +50,14 @@ func TestGeneratePassword_Shape(t *testing.T) { } func TestValidateRealmName(t *testing.T) { - valid := []string{"a", "vapi", "vapi-test", "a1", "abc123def456ghi789jkl012mno34"} + valid := []string{"a", "vapi", "a1", "abc123def456ghi789jkl012mno34"} for _, n := range valid { if err := ValidateRealmName(n); err != nil { t.Errorf("ValidateRealmName(%q) = %v, want nil", n, err) } } - invalid := []string{"", "-vapi", "vapi-", "va pi", "vapi.test", "VAPI_TEST", - "abcdefghij0123456789abcdefghij0"} // 31 chars + invalid := []string{"", "vapi-test", "-vapi", "vapi-", "va pi", "vapi.test", + "VAPI", "VAPI_TEST", "abcdefghij0123456789abcdefghij0"} // 31 chars for _, n := range invalid { if err := ValidateRealmName(n); err == nil { t.Errorf("ValidateRealmName(%q) = nil, want error", n)