diff --git a/rest-api/AGENTS.md b/rest-api/AGENTS.md index 102882dc75..760b77a243 100644 --- a/rest-api/AGENTS.md +++ b/rest-api/AGENTS.md @@ -814,6 +814,21 @@ All commits **must** meet the following signing requirement: - Keep PRs focused on a single change. - Do not land unused code unless the PR is too large to review otherwise. - Ensure all CI checks pass before requesting review. +- Before requesting review for a Go change, run `make lint-go` and inspect its + complete analyzer output. Its `golangci-lint` command uses + `--issues-exit-code 0`, so a successful command does not mean the output is + clean. Run `go tool golangci-lint run ` without that + override and fix every finding in the changed package. +- When CLI flags override configuration, copy the complete configured object + first and overlay only explicitly set flags. Test no override, one override, + and unset configured fields so unrelated options cannot be discarded. +- Before using an authentication or protocol convenience API, read its current + documentation for protocol-specific encoding rules. Exercise delimiters, + percent signs, plus signs, and other reserved characters through the real + encode and decode boundary. +- Before requesting review, group every changed Go function's scenarios under + one table-driven top-level test. Treat scenario-specific top-level tests as a + review failure even when the test suite passes. ## CI / CD diff --git a/rest-api/cli/README.md b/rest-api/cli/README.md index 15aba0a81f..290b5b8bad 100644 --- a/rest-api/cli/README.md +++ b/rest-api/cli/README.md @@ -127,6 +127,10 @@ auth: token_url: http://localhost:8080/realms/nico-dev/protocol/openid-connect/token client_id: nico-api client_secret: nico-local-secret + # scopes: [openid] + # client_auth_method: client_secret_post + # token_parameters: + # audience: nico # Run `nicocli login` to authenticate; it will prompt for username/password # and persist the resulting bearer token (and refresh token) here. @@ -159,7 +163,7 @@ These flags apply to every command and override the corresponding config values. ### Configuring with environment variables -Every field in `~/.nico/config.yaml` can also be set via a `NICO_*` environment variable. When both a config value and an env var are present, the env var wins; an explicit command-line flag still beats both. Set any of these in your shell instead of editing the config file: +The scalar fields listed below can also be set via a `NICO_*` environment variable. When both a config value and an env var are present, the env var wins; an explicit command-line flag still beats both. Set any of these in your shell instead of editing the config file: | Env Var | Config field | Notes | |---------|--------------|-------| @@ -183,6 +187,13 @@ Every field in `~/.nico/config.yaml` can also be set via a `NICO_*` environment `NICO_KEYCLOAK_URL` and `NICO_KEYCLOAK_REALM` do not map to a single config field; they feed the login command and construct the OIDC `token_url` at login time. +Client-credentials configurations can also set `auth.oidc.scopes` as a YAML list, +`auth.oidc.token_parameters` as a map of additional non-secret form parameters, +and `auth.oidc.client_auth_method` to `client_secret_post` or +`client_secret_basic`. The default remains `scope=openid` with +`client_secret_post`. Reserved OAuth and credential fields cannot be overridden +through `token_parameters`. + To see exactly which `NICO_*` variables are in use right now, pass `--debug` on any command: ```bash diff --git a/rest-api/cli/pkg/auth.go b/rest-api/cli/pkg/auth.go index b815f6767e..fb141b2b30 100644 --- a/rest-api/cli/pkg/auth.go +++ b/rest-api/cli/pkg/auth.go @@ -239,7 +239,7 @@ func LoginWithOIDCConfig(cfg *ConfigFile, configPath string) (string, error) { tokenResp, err = refreshTokenGrant(oidc.TokenURL, oidc.ClientID, oidc.ClientSecret, oidc.RefreshToken) } if tokenResp == nil && oidc.Username == "" && oidc.ClientSecret != "" { - tokenResp, err = clientCredentialsGrant(oidc.TokenURL, oidc.ClientID, oidc.ClientSecret) + tokenResp, err = clientCredentialsGrant(oidc) } if tokenResp == nil && oidc.Username != "" && oidc.Password != "" { tokenResp, err = passwordGrant(oidc.TokenURL, oidc.ClientID, oidc.ClientSecret, oidc.Username, oidc.Password) @@ -393,7 +393,7 @@ func loginWithOIDCCmd(c *cli.Context, cfg *ConfigFile) error { } clientID := c.String("client-id") - if clientID == "" && cfg.Auth.OIDC != nil { + if cfg.Auth.OIDC != nil && cfg.Auth.OIDC.ClientID != "" && !cliFlagExplicitlySet(c, "client-id") { clientID = cfg.Auth.OIDC.ClientID } @@ -416,11 +416,20 @@ func loginWithOIDCCmd(c *cli.Context, cfg *ConfigFile) error { var err error if username == "" && clientSecret != "" { - tokenResp, err = clientCredentialsGrant(tokenURL, clientID, clientSecret) + requestOIDC := ConfigOIDC{} + if cfg.Auth.OIDC != nil { + requestOIDC = *cfg.Auth.OIDC + } + requestOIDC.TokenURL = tokenURL + requestOIDC.ClientID = clientID + requestOIDC.ClientSecret = clientSecret + tokenResp, err = clientCredentialsGrant(&requestOIDC) } else { if username == "" { fmt.Print("Username: ") - fmt.Scanln(&username) + if _, scanErr := fmt.Scanln(&username); scanErr != nil { + return fmt.Errorf("reading username: %w", scanErr) + } } if password == "" { fmt.Print("Password: ") @@ -468,14 +477,59 @@ func passwordGrant(tokenURL, clientID, clientSecret, username, password string) return postToken(tokenURL, data) } -func clientCredentialsGrant(tokenURL, clientID, clientSecret string) (*TokenResponse, error) { +func clientCredentialsGrant(oidc *ConfigOIDC) (*TokenResponse, error) { + const ( + clientSecretPost = "client_secret_post" + clientSecretBasic = "client_secret_basic" + ) + reserved := map[string]struct{}{ + "grant_type": {}, "client_id": {}, "client_secret": {}, "scope": {}, + "username": {}, "password": {}, "refresh_token": {}, + "client_assertion": {}, "client_assertion_type": {}, + } data := url.Values{ - "grant_type": {"client_credentials"}, - "client_id": {clientID}, - "client_secret": {clientSecret}, - "scope": {"openid"}, + "grant_type": {"client_credentials"}, + } + scopes := oidc.Scopes + if len(scopes) == 0 { + scopes = []string{"openid"} + } + data.Set("scope", strings.Join(scopes, " ")) + for name, value := range oidc.TokenParameters { + if _, blocked := reserved[name]; blocked { + return nil, fmt.Errorf("reserved token parameter %q cannot be configured", name) + } + data.Set(name, value) + } + + method := oidc.ClientAuthMethod + if method == "" { + method = clientSecretPost + } + switch method { + case clientSecretPost: + data.Set("client_id", oidc.ClientID) + data.Set("client_secret", oidc.ClientSecret) + return postToken(oidc.TokenURL, data) + case clientSecretBasic: + return postTokenWithBasicAuth(oidc.TokenURL, data, oidc.ClientID, oidc.ClientSecret) + default: + return nil, fmt.Errorf("unsupported client_auth_method %q", method) } - return postToken(tokenURL, data) +} + +func postTokenWithBasicAuth(tokenURL string, data url.Values, clientID, clientSecret string) (*TokenResponse, error) { + req, err := http.NewRequest(http.MethodPost, tokenURL, strings.NewReader(data.Encode())) + if err != nil { + return nil, fmt.Errorf("token request: %w", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.SetBasicAuth(url.QueryEscape(clientID), url.QueryEscape(clientSecret)) + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, fmt.Errorf("token request: %w", err) + } + return parseTokenResponse(resp) } func refreshTokenGrant(tokenURL, clientID, clientSecret, refreshToken string) (*TokenResponse, error) { @@ -495,6 +549,10 @@ func postToken(tokenURL string, data url.Values) (*TokenResponse, error) { if err != nil { return nil, fmt.Errorf("token request: %w", err) } + return parseTokenResponse(resp) +} + +func parseTokenResponse(resp *http.Response) (*TokenResponse, error) { defer resp.Body.Close() if resp.StatusCode != 200 { diff --git a/rest-api/cli/pkg/auth_test.go b/rest-api/cli/pkg/auth_test.go index b92046694a..888c320edf 100644 --- a/rest-api/cli/pkg/auth_test.go +++ b/rest-api/cli/pkg/auth_test.go @@ -7,6 +7,7 @@ import ( "flag" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" "strconv" @@ -17,6 +18,152 @@ import ( cli "github.com/urfave/cli/v2" ) +func TestLoginWithOIDCConfig(t *testing.T) { + tests := []struct { + name string + oidc ConfigOIDC + checkRequest func(*testing.T, *http.Request) + wantErr string + wantRequests int + wantSavedToken bool + }{ + { + name: "custom client credentials request", + oidc: ConfigOIDC{ + ClientID: "client:id", + ClientSecret: "client+secret%", + Scopes: []string{"carbide", "offline_access"}, + TokenParameters: map[string]string{"audience": "nico"}, + ClientAuthMethod: "client_secret_basic", + }, + checkRequest: func(t *testing.T, r *http.Request) { + require.NoError(t, r.ParseForm()) + require.Equal(t, "client_credentials", r.Form.Get("grant_type")) + require.Equal(t, "carbide offline_access", r.Form.Get("scope")) + require.Equal(t, "nico", r.Form.Get("audience")) + require.Empty(t, r.Form.Get("client_id")) + require.Empty(t, r.Form.Get("client_secret")) + clientID, clientSecret, ok := r.BasicAuth() + require.True(t, ok) + decodedClientID, err := url.QueryUnescape(clientID) + require.NoError(t, err) + decodedClientSecret, err := url.QueryUnescape(clientSecret) + require.NoError(t, err) + require.Equal(t, "client:id", decodedClientID) + require.Equal(t, "client+secret%", decodedClientSecret) + }, + wantRequests: 1, + wantSavedToken: true, + }, + { + name: "default client credentials request", + oidc: ConfigOIDC{ClientID: "client-id", ClientSecret: "client-secret"}, + checkRequest: func(t *testing.T, r *http.Request) { + require.NoError(t, r.ParseForm()) + require.Equal(t, "client_credentials", r.Form.Get("grant_type")) + require.Equal(t, "openid", r.Form.Get("scope")) + require.Equal(t, "client-id", r.Form.Get("client_id")) + require.Equal(t, "client-secret", r.Form.Get("client_secret")) + require.Empty(t, r.Header.Get("Authorization")) + }, + wantRequests: 1, + }, + { + name: "reserved token parameter", + oidc: ConfigOIDC{ + ClientID: "client-id", + ClientSecret: "client-secret", + TokenParameters: map[string]string{"client_secret": "replacement"}, + }, + wantErr: "reserved token parameter", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + requests := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + tt.checkRequest(t, r) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"new-token","expires_in":3600}`)) + })) + defer server.Close() + + tt.oidc.TokenURL = server.URL + cfg := &ConfigFile{Auth: ConfigAuth{OIDC: &tt.oidc}} + configPath := filepath.Join(t.TempDir(), "config.yaml") + token, err := LoginWithOIDCConfig(cfg, configPath) + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + } else { + require.NoError(t, err) + require.Equal(t, "new-token", token) + } + require.Equal(t, tt.wantRequests, requests) + if tt.wantSavedToken { + loaded, err := LoadConfigFromPath(configPath) + require.NoError(t, err) + require.Equal(t, "new-token", loaded.Auth.OIDC.Token) + } + }) + } +} + +func TestLoginWithOIDCCmd(t *testing.T) { + tests := []struct { + name string + clientIDArgs []string + wantClientID string + }{ + { + name: "preserves configured client ID", + wantClientID: "client-id", + }, + { + name: "uses explicit client ID", + clientIDArgs: []string{"--client-id", "override-id"}, + wantClientID: "override-id", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + require.Equal(t, "carbide", r.Form.Get("scope")) + require.Equal(t, "nico", r.Form.Get("audience")) + clientID, clientSecret, ok := r.BasicAuth() + require.True(t, ok) + require.Equal(t, tt.wantClientID, clientID) + require.Equal(t, "client-secret", clientSecret) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"new-token","expires_in":3600}`)) + })) + defer server.Close() + + configPath := filepath.Join(t.TempDir(), "config.yaml") + cfg := &ConfigFile{Auth: ConfigAuth{OIDC: &ConfigOIDC{ + TokenURL: "https://auth.example.invalid/token", + ClientID: "client-id", + ClientSecret: "client-secret", + Scopes: []string{"carbide"}, + TokenParameters: map[string]string{"audience": "nico"}, + ClientAuthMethod: "client_secret_basic", + }}} + require.NoError(t, SaveConfigToPath(cfg, configPath)) + SetConfigPath(configPath) + defer SetConfigPath("") + app, err := NewApp([]byte(`{"openapi":"3.0.0","info":{"title":"test","version":"test"},"paths":{}}`)) + require.NoError(t, err) + args := append([]string{"nicocli", "--token-url", server.URL}, tt.clientIDArgs...) + args = append(args, "login") + withArgs(t, args...) + require.NoError(t, app.Run(os.Args)) + }) + } +} + func TestExtractNGCToken(t *testing.T) { tests := []struct { name string diff --git a/rest-api/cli/pkg/config.go b/rest-api/cli/pkg/config.go index 6d23245983..0acbe18ace 100644 --- a/rest-api/cli/pkg/config.go +++ b/rest-api/cli/pkg/config.go @@ -31,14 +31,17 @@ type ConfigAuth struct { } type ConfigOIDC struct { - TokenURL string `yaml:"token_url,omitempty"` - ClientID string `yaml:"client_id,omitempty"` - ClientSecret string `yaml:"client_secret,omitempty"` - Username string `yaml:"username,omitempty"` - Password string `yaml:"password,omitempty"` - Token string `yaml:"token,omitempty"` - RefreshToken string `yaml:"refresh_token,omitempty"` - ExpiresAt string `yaml:"expires_at,omitempty"` + TokenURL string `yaml:"token_url,omitempty"` + ClientID string `yaml:"client_id,omitempty"` + ClientSecret string `yaml:"client_secret,omitempty"` + ClientAuthMethod string `yaml:"client_auth_method,omitempty"` + Scopes []string `yaml:"scopes,omitempty"` + TokenParameters map[string]string `yaml:"token_parameters,omitempty"` + Username string `yaml:"username,omitempty"` + Password string `yaml:"password,omitempty"` + Token string `yaml:"token,omitempty"` + RefreshToken string `yaml:"refresh_token,omitempty"` + ExpiresAt string `yaml:"expires_at,omitempty"` } type ConfigAPIKey struct { @@ -113,7 +116,9 @@ func SaveConfigToPath(cfg *ConfigFile, path string) error { // Load existing file as raw map to preserve unknown keys. raw := make(map[string]interface{}) if existing, err := os.ReadFile(path); err == nil { - yaml.Unmarshal(existing, &raw) + if err := yaml.Unmarshal(existing, &raw); err != nil { + return fmt.Errorf("parsing existing config %s: %w", path, err) + } } // Marshal the struct and merge into the raw map. @@ -122,7 +127,9 @@ func SaveConfigToPath(cfg *ConfigFile, path string) error { return fmt.Errorf("marshaling config: %w", err) } var cfgMap map[string]interface{} - yaml.Unmarshal(structured, &cfgMap) + if err := yaml.Unmarshal(structured, &cfgMap); err != nil { + return fmt.Errorf("parsing marshaled config: %w", err) + } for k, v := range cfgMap { raw[k] = v } @@ -198,6 +205,10 @@ auth: token_url: http://localhost:8080/realms/nico-dev/protocol/openid-connect/token client_id: nico-api client_secret: nico-local-secret + # scopes: [openid] + # client_auth_method: client_secret_post + # token_parameters: + # audience: nico # Run 'nicocli login' to authenticate; it will prompt for username/password # and persist the resulting bearer token (and refresh token) here. diff --git a/rest-api/cli/pkg/config_test.go b/rest-api/cli/pkg/config_test.go index 528eec8924..64d81a4c8e 100644 --- a/rest-api/cli/pkg/config_test.go +++ b/rest-api/cli/pkg/config_test.go @@ -168,42 +168,33 @@ func TestHasAPIKeyConfig(t *testing.T) { } } -func TestSaveConfigPreservesUnknownKeys(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "config.yaml") +func TestSaveConfigToPath(t *testing.T) { + t.Run("preserves unknown keys", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + initial := "api:\n base: http://localhost\ncustom_key: my-value\n" + require.NoError(t, os.WriteFile(path, []byte(initial), 0600)) - // Write a config with a custom key. - initial := "api:\n base: http://localhost\ncustom_key: my-value\n" - if err := os.WriteFile(path, []byte(initial), 0600); err != nil { - t.Fatal(err) - } + cfg, err := LoadConfigFromPath(path) + require.NoError(t, err) + cfg.API.Org = "test-org" + require.NoError(t, SaveConfigToPath(cfg, path)) - // Load, modify, and save. - cfg, err := LoadConfigFromPath(path) - if err != nil { - t.Fatal(err) - } - cfg.API.Org = "test-org" - if err := SaveConfigToPath(cfg, path); err != nil { - t.Fatal(err) - } + data, err := os.ReadFile(path) + require.NoError(t, err) + content := string(data) + require.Contains(t, content, "custom_key") + require.Contains(t, content, "test-org") + require.Contains(t, content, "http://localhost") + }) - // Read back and verify custom key is preserved. - data, err := os.ReadFile(path) - if err != nil { - t.Fatal(err) - } - content := string(data) + t.Run("rejects malformed existing config", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "config.yaml") + require.NoError(t, os.WriteFile(path, []byte("api: ["), 0600)) - if !contains(content, "custom_key") { - t.Errorf("SaveConfigToPath lost unknown key 'custom_key'. Content:\n%s", content) - } - if !contains(content, "test-org") { - t.Errorf("SaveConfigToPath lost org value. Content:\n%s", content) - } - if !contains(content, "http://localhost") { - t.Errorf("SaveConfigToPath lost base URL. Content:\n%s", content) - } + err := SaveConfigToPath(&ConfigFile{}, path) + require.ErrorContains(t, err, "parsing existing config") + }) } func TestLoadConfigFromPath_NotFound(t *testing.T) { @@ -228,12 +219,15 @@ func TestLoadConfigFromPath_Roundtrip(t *testing.T) { }, Auth: ConfigAuth{ OIDC: &ConfigOIDC{ - TokenURL: "http://localhost:8080/realms/nico-dev/protocol/openid-connect/token", - ClientID: "nico-api", - ClientSecret: "secret", - Token: "eyJhbG...", - RefreshToken: "refresh...", - ExpiresAt: "2026-01-01T00:00:00Z", + TokenURL: "http://localhost:8080/realms/nico-dev/protocol/openid-connect/token", + ClientID: "nico-api", + ClientSecret: "secret", + ClientAuthMethod: "client_secret_basic", + Scopes: []string{"carbide", "offline_access"}, + TokenParameters: map[string]string{"audience": "nico"}, + Token: "eyJhbG...", + RefreshToken: "refresh...", + ExpiresAt: "2026-01-01T00:00:00Z", }, }, } @@ -262,6 +256,9 @@ func TestLoadConfigFromPath_Roundtrip(t *testing.T) { if loaded.Auth.OIDC.ClientSecret != original.Auth.OIDC.ClientSecret { t.Errorf("Auth.OIDC.ClientSecret = %q, want %q", loaded.Auth.OIDC.ClientSecret, original.Auth.OIDC.ClientSecret) } + require.Equal(t, original.Auth.OIDC.ClientAuthMethod, loaded.Auth.OIDC.ClientAuthMethod) + require.Equal(t, original.Auth.OIDC.Scopes, loaded.Auth.OIDC.Scopes) + require.Equal(t, original.Auth.OIDC.TokenParameters, loaded.Auth.OIDC.TokenParameters) } func contains(s, substr string) bool { diff --git a/rest-api/cli/pkg/site_bootstrap.go b/rest-api/cli/pkg/site_bootstrap.go index aba15927b7..1a3559c4bc 100644 --- a/rest-api/cli/pkg/site_bootstrap.go +++ b/rest-api/cli/pkg/site_bootstrap.go @@ -1027,7 +1027,7 @@ func (references bootstrapReferences) resolve(value any) (any, error) { if err != nil { return nil, err } - result.WriteString(fmt.Sprint(resolved)) + fmt.Fprint(&result, resolved) last = match[1] } result.WriteString(typed[last:]) diff --git a/rest-api/cli/tui/commands.go b/rest-api/cli/tui/commands.go index 6233f7268e..d396f85526 100644 --- a/rest-api/cli/tui/commands.go +++ b/rest-api/cli/tui/commands.go @@ -388,7 +388,9 @@ func cmdSiteCreate(s *Session, _ []string) error { s.Cache.Invalidate("site") s.Cache.InvalidateFiltered() var created map[string]interface{} - json.Unmarshal(resp, &created) + if err := json.Unmarshal(resp, &created); err != nil { + return fmt.Errorf("parsing created site: %w", err) + } fmt.Printf("%s Site created: %s (%s)\n", Green("OK"), str(created, "name"), str(created, "id")) return nil } @@ -501,7 +503,9 @@ func cmdSiteUpdate(s *Session, args []string) error { s.Cache.Invalidate("site") s.Cache.InvalidateFiltered() var updated map[string]interface{} - json.Unmarshal(resp, &updated) + if err := json.Unmarshal(resp, &updated); err != nil { + return fmt.Errorf("parsing updated site: %w", err) + } fmt.Printf("%s Site updated: %s (%s)\n", Green("OK"), str(updated, "name"), str(updated, "id")) return nil } @@ -598,7 +602,9 @@ func cmdVPCCreate(s *Session, _ []string) error { } s.Cache.Invalidate("vpc") var created map[string]interface{} - json.Unmarshal(resp, &created) + if err := json.Unmarshal(resp, &created); err != nil { + return fmt.Errorf("parsing created VPC: %w", err) + } fmt.Printf("%s VPC created: %s (%s)\n", Green("OK"), str(created, "name"), str(created, "id")) return nil } @@ -635,7 +641,9 @@ func cmdVPCUpdate(s *Session, args []string) error { s.Cache.Invalidate("vpc") s.Cache.InvalidateFiltered() var updated map[string]interface{} - json.Unmarshal(resp, &updated) + if err := json.Unmarshal(resp, &updated); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s VPC updated: %s (%s)\n", Green("OK"), str(updated, "name"), str(updated, "id")) return nil } @@ -678,7 +686,9 @@ func cmdVPCVirtualizationUpdate(s *Session, args []string) error { s.Cache.Invalidate("vpc") s.Cache.InvalidateFiltered() var updated map[string]interface{} - json.Unmarshal(resp, &updated) + if err := json.Unmarshal(resp, &updated); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s VPC virtualization update submitted: %s (%s)\n", Green("OK"), str(updated, "name"), str(updated, "id")) return nil } @@ -738,8 +748,10 @@ func cmdSubnetCreate(s *Session, _ []string) error { if err != nil { return err } - var prefixLen int - fmt.Sscanf(prefixLenText, "%d", &prefixLen) + prefixLen, err := strconv.Atoi(prefixLenText) + if err != nil { + return fmt.Errorf("prefix length must be an integer: %w", err) + } if prefixLen < 1 || prefixLen > 32 { return fmt.Errorf("prefix length must be between 1 and 32") } @@ -784,7 +796,9 @@ func cmdSubnetCreate(s *Session, _ []string) error { s.Cache.Invalidate("subnet") s.Cache.InvalidateFiltered() var created map[string]interface{} - json.Unmarshal(resp, &created) + if err := json.Unmarshal(resp, &created); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s Subnet created: %s (%s)\n", Green("OK"), str(created, "name"), str(created, "id")) return nil } @@ -821,7 +835,9 @@ func cmdSubnetUpdate(s *Session, args []string) error { s.Cache.Invalidate("subnet") s.Cache.InvalidateFiltered() var updated map[string]interface{} - json.Unmarshal(resp, &updated) + if err := json.Unmarshal(resp, &updated); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s Subnet updated: %s (%s)\n", Green("OK"), str(updated, "name"), str(updated, "id")) return nil } @@ -1128,7 +1144,9 @@ func cmdOSCreate(s *Session, _ []string) error { s.Cache.Invalidate("operating-system") s.Cache.InvalidateFiltered() var created map[string]interface{} - json.Unmarshal(resp, &created) + if err := json.Unmarshal(resp, &created); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s Operating system created: %s (%s)\n", Green("OK"), str(created, "name"), str(created, "id")) return nil } @@ -1210,7 +1228,9 @@ func cmdOSUpdate(s *Session, args []string) error { s.Cache.Invalidate("operating-system") s.Cache.InvalidateFiltered() var updated map[string]interface{} - json.Unmarshal(resp, &updated) + if err := json.Unmarshal(resp, &updated); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s Operating system updated: %s (%s)\n", Green("OK"), str(updated, "name"), str(updated, "id")) return nil } @@ -1284,7 +1304,9 @@ func cmdSSHKeyGroupCreate(s *Session, _ []string) error { s.Cache.Invalidate("ssh-key-group") s.Cache.InvalidateFiltered() var created map[string]interface{} - json.Unmarshal(resp, &created) + if err := json.Unmarshal(resp, &created); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s SSH key group created: %s (%s)\n", Green("OK"), str(created, "name"), str(created, "id")) return nil } @@ -1343,7 +1365,9 @@ func cmdSSHKeyGroupUpdate(s *Session, args []string) error { s.Cache.Invalidate("ssh-key-group") s.Cache.InvalidateFiltered() var updated map[string]interface{} - json.Unmarshal(resp, &updated) + if err := json.Unmarshal(resp, &updated); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s SSH key group updated: %s (%s)\n", Green("OK"), str(updated, "name"), str(updated, "id")) return nil } @@ -1413,7 +1437,9 @@ func cmdSSHKeyCreate(s *Session, _ []string) error { s.Cache.Invalidate("ssh-key-group") s.Cache.InvalidateFiltered() var created map[string]interface{} - json.Unmarshal(resp, &created) + if err := json.Unmarshal(resp, &created); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s SSH key created: %s (%s)\n", Green("OK"), str(created, "name"), str(created, "id")) return nil } @@ -1440,7 +1466,9 @@ func cmdSSHKeyUpdate(s *Session, args []string) error { s.Cache.Invalidate("ssh-key-group") s.Cache.InvalidateFiltered() var updated map[string]interface{} - json.Unmarshal(resp, &updated) + if err := json.Unmarshal(resp, &updated); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s SSH key updated: %s (%s)\n", Green("OK"), str(updated, "name"), str(updated, "id")) return nil } @@ -1534,7 +1562,9 @@ func cmdAllocationCreate(s *Session, _ []string) error { s.Cache.Invalidate("allocation") s.Cache.InvalidateFiltered() var created map[string]interface{} - json.Unmarshal(resp, &created) + if err := json.Unmarshal(resp, &created); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s Allocation created: %s (%s)\n", Green("OK"), str(created, "name"), str(created, "id")) return nil } @@ -1836,7 +1866,9 @@ func cmdAllocationUpdate(s *Session, args []string) error { s.Cache.Invalidate("allocation") s.Cache.InvalidateFiltered() var updated map[string]interface{} - json.Unmarshal(resp, &updated) + if err := json.Unmarshal(resp, &updated); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s Allocation updated: %s (%s)\n", Green("OK"), str(updated, "name"), str(updated, "id")) return nil } @@ -1948,7 +1980,9 @@ func cmdIPBlockCreate(s *Session, _ []string) error { } s.Cache.Invalidate("ip-block") var created map[string]interface{} - json.Unmarshal(resp, &created) + if err := json.Unmarshal(resp, &created); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s IP block created: %s (%s)\n", Green("OK"), str(created, "name"), str(created, "id")) return nil } @@ -1985,7 +2019,9 @@ func cmdIPBlockUpdate(s *Session, args []string) error { s.Cache.Invalidate("ip-block") s.Cache.InvalidateFiltered() var updated map[string]interface{} - json.Unmarshal(resp, &updated) + if err := json.Unmarshal(resp, &updated); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s IP block updated: %s (%s)\n", Green("OK"), str(updated, "name"), str(updated, "id")) return nil } @@ -2063,7 +2099,9 @@ func cmdNSGCreate(s *Session, _ []string) error { s.Cache.Invalidate("network-security-group") s.Cache.InvalidateFiltered() var created map[string]interface{} - json.Unmarshal(resp, &created) + if err := json.Unmarshal(resp, &created); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s Network security group created: %s (%s)\n", Green("OK"), str(created, "name"), str(created, "id")) return nil } @@ -2100,7 +2138,9 @@ func cmdNSGUpdate(s *Session, args []string) error { s.Cache.Invalidate("network-security-group") s.Cache.InvalidateFiltered() var updated map[string]interface{} - json.Unmarshal(resp, &updated) + if err := json.Unmarshal(resp, &updated); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s Network security group updated: %s (%s)\n", Green("OK"), str(updated, "name"), str(updated, "id")) return nil } @@ -2210,8 +2250,10 @@ func cmdVPCPrefixCreate(s *Session, _ []string) error { if err != nil { return err } - var prefixLen int - fmt.Sscanf(prefixLenText, "%d", &prefixLen) + prefixLen, err := strconv.Atoi(prefixLenText) + if err != nil { + return fmt.Errorf("prefix length must be an integer: %w", err) + } if prefixLen < 8 || prefixLen > 31 { return fmt.Errorf("prefix length must be between 8 and 31") } @@ -2237,7 +2279,9 @@ func cmdVPCPrefixCreate(s *Session, _ []string) error { s.Cache.Invalidate("vpc-prefix") s.Cache.InvalidateFiltered() var created map[string]interface{} - json.Unmarshal(resp, &created) + if err := json.Unmarshal(resp, &created); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s VPC prefix created: %s (%s)\n", Green("OK"), str(created, "name"), str(created, "id")) return nil } @@ -2329,7 +2373,9 @@ func cmdVPCPrefixUpdate(s *Session, args []string) error { s.Cache.Invalidate("vpc-prefix") s.Cache.InvalidateFiltered() var updated map[string]interface{} - json.Unmarshal(resp, &updated) + if err := json.Unmarshal(resp, &updated); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s VPC prefix updated: %s (%s)\n", Green("OK"), str(updated, "name"), str(updated, "id")) return nil } @@ -2397,7 +2443,9 @@ func cmdTenantAccountCreate(s *Session, _ []string) error { } s.Cache.Invalidate("tenant-account") var created map[string]interface{} - json.Unmarshal(resp, &created) + if err := json.Unmarshal(resp, &created); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s Tenant account created: %s (%s)\n", Green("OK"), str(created, "tenantOrg"), str(created, "id")) return nil } @@ -2419,7 +2467,9 @@ func cmdTenantAccountUpdate(s *Session, args []string) error { } s.Cache.Invalidate("tenant-account") var updated map[string]interface{} - json.Unmarshal(resp, &updated) + if err := json.Unmarshal(resp, &updated); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s Tenant account accepted: %s (%s)\n", Green("OK"), str(updated, "tenantOrg"), str(updated, "id")) return nil } @@ -2762,7 +2812,9 @@ func cmdInstanceCreate(s *Session, _ []string) error { s.Cache.Invalidate("instance") s.Cache.InvalidateFiltered() var created map[string]interface{} - json.Unmarshal(resp, &created) + if err := json.Unmarshal(resp, &created); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s Instance created: %s (%s)\n", Green("OK"), str(created, "name"), str(created, "id")) return nil } @@ -2975,7 +3027,9 @@ func cmdInstanceUpdate(s *Session, args []string) error { s.Cache.Invalidate("instance") s.Cache.InvalidateFiltered() var updated map[string]interface{} - json.Unmarshal(resp, &updated) + if err := json.Unmarshal(resp, &updated); err != nil { + return fmt.Errorf("parsing response: %w", err) + } fmt.Printf("%s Instance updated: %s (%s)\n", Green("OK"), str(updated, "name"), str(updated, "id")) return nil } @@ -4256,7 +4310,8 @@ func sortByLabelKey(items []NamedItem, key string) []NamedItem { func parseLabelArgs(args []string) (remaining []string, labels map[string]string, sortKey string, err error) { labels = map[string]string{} for i := 0; i < len(args); i++ { - if args[i] == "--label" { + switch args[i] { + case "--label": if i+1 >= len(args) { return nil, nil, "", fmt.Errorf("--label requires a key=value argument") } @@ -4269,13 +4324,13 @@ func parseLabelArgs(args []string) (remaining []string, labels map[string]string } else { return nil, nil, "", fmt.Errorf("--label value %q must contain '='", args[i]) } - } else if args[i] == "--sort-label" { + case "--sort-label": if i+1 >= len(args) { return nil, nil, "", fmt.Errorf("--sort-label requires a key argument") } i++ sortKey = args[i] - } else { + default: remaining = append(remaining, args[i]) } } diff --git a/rest-api/cli/tui/commands_test.go b/rest-api/cli/tui/commands_test.go index 7fee859e8b..4197fe25cd 100644 --- a/rest-api/cli/tui/commands_test.go +++ b/rest-api/cli/tui/commands_test.go @@ -1256,7 +1256,9 @@ func captureStdout(f func()) string { w.Close() os.Stdout = old var buf bytes.Buffer - io.Copy(&buf, r) + if _, err := io.Copy(&buf, r); err != nil { + panic(err) + } return buf.String() } diff --git a/rest-api/cli/tui/generated_commands.go b/rest-api/cli/tui/generated_commands.go index 888f377139..a100c443c4 100644 --- a/rest-api/cli/tui/generated_commands.go +++ b/rest-api/cli/tui/generated_commands.go @@ -767,10 +767,10 @@ func logGeneratedCommand(s *Session, info appcli.GeneratedCommandInfo, args []st func quoteShellCommandArgument(value string) string { if value != "" && strings.IndexFunc(value, func(char rune) bool { - return !(char >= 'a' && char <= 'z' || - char >= 'A' && char <= 'Z' || - char >= '0' && char <= '9' || - strings.ContainsRune("_@%+=:,./-", char)) + return (char < 'a' || char > 'z') && + (char < 'A' || char > 'Z') && + (char < '0' || char > '9') && + !strings.ContainsRune("_@%+=:,./-", char) }) == -1 { return value } diff --git a/rest-api/cli/tui/repl.go b/rest-api/cli/tui/repl.go index 54e4aa3353..950669f5de 100644 --- a/rest-api/cli/tui/repl.go +++ b/rest-api/cli/tui/repl.go @@ -95,7 +95,6 @@ var argResourceMap = map[string]string{ } var history []string -var historyPos int // RunREPL starts the interactive REPL loop with inline autocomplete. func RunREPL(s *Session) error { @@ -358,7 +357,6 @@ func readLineWithSuggestions(s *Session, cmdNames []string) (string, error) { prompt := s.PromptString() line := "" - historyPos = -1 selectedSuggestion := -1 prevSuggestionCount := 0 @@ -409,7 +407,6 @@ func readLineWithSuggestions(s *Session, cmdNames []string) (string, error) { case key.Char == KeyCtrlC: line = "" selectedSuggestion = -1 - historyPos = -1 clearSuggestionLines(prevSuggestionCount) prevSuggestionCount = 0 renderInput() @@ -426,7 +423,6 @@ func readLineWithSuggestions(s *Session, cmdNames []string) (string, error) { if selectedSuggestion >= 0 && selectedSuggestion < len(suggestions) { line = suggestions[selectedSuggestion] selectedSuggestion = -1 - historyPos = -1 clearSuggestionLines(prevSuggestionCount) prevSuggestionCount = 0 renderInput() @@ -435,7 +431,6 @@ func readLineWithSuggestions(s *Session, cmdNames []string) (string, error) { clearSuggestionLines(prevSuggestionCount) ClearLine() fmt.Print("\r" + prompt + line + "\r\n") - historyPos = -1 return line, nil case key.Char == '\t': @@ -484,7 +479,6 @@ func readLineWithSuggestions(s *Session, cmdNames []string) (string, error) { line = chosen } selectedSuggestion = -1 - historyPos = -1 } renderInput() @@ -507,14 +501,12 @@ func readLineWithSuggestions(s *Session, cmdNames []string) (string, error) { if len(line) > 0 { line = line[:len(line)-1] selectedSuggestion = -1 - historyPos = -1 } renderInput() case key.Char >= 32 && key.Char < 127: line += string(key.Char) selectedSuggestion = -1 - historyPos = -1 renderInput() default: @@ -806,7 +798,7 @@ func runScopeSet(s *Session, resourceType, nameOrID string) { return } } else { - item, err = s.Resolver.Resolve(context.Background(), resourceType, strings.Title(resourceType)) + item, err = s.Resolver.Resolve(context.Background(), resourceType, strings.ToUpper(resourceType[:1])+resourceType[1:]) if err != nil { fmt.Fprintf(os.Stderr, "%s %v\n\n", Red("Error:"), err) return diff --git a/rest-api/cli/tui/term.go b/rest-api/cli/tui/term.go index e8bfa8ff3c..ed8dc1796e 100644 --- a/rest-api/cli/tui/term.go +++ b/rest-api/cli/tui/term.go @@ -41,7 +41,9 @@ func RawMode() (restore func(), err error) { return nil, fmt.Errorf("entering raw mode: %w", err) } return func() { - term.Restore(fd, oldState) + if restoreErr := term.Restore(fd, oldState); restoreErr != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to restore terminal mode: %v\n", restoreErr) + } }, nil }