FR-6398: Harden the frbit-cli release pipeline after the security review

.github/dependabot.yml

@@ -0,0 +1,14 @@
+version: 2
+updates:
+  - package-ecosystem: gomod
+    directory: /
+    schedule:
+      interval: weekly
+  - package-ecosystem: github-actions
+    directory: /
+    schedule:
+      interval: weekly
+  - package-ecosystem: npm
+    directory: /npm
+    schedule:
+      interval: weekly

.github/workflows/release.yml

@@ -6,6 +6,8 @@ on:
       - "v*"
 
 permissions:
+  attestations: write
+  artifact-metadata: write
   contents: write
   id-token: write
 
@@ -14,6 +16,7 @@ env:
 
 jobs:
   release:
+    environment: release
     runs-on: ubuntu-24.04
     steps:
       - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6
@@ -39,6 +42,10 @@ jobs:
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           HOMEBREW_TAP_TOKEN: ${{ secrets.HOMEBREW_TAP_TOKEN }}
+      - name: Attest release checksums
+        uses: actions/attest@1e69f48acb82d1966a394da916b4c1698aa569d6 # v4
+        with:
+          subject-path: dist/checksums.txt
       - name: Prepare npm packages
         if: env.PUBLISH_NPM == 'true'
         run: node npm/scripts/prepare-release.mjs "${GITHUB_REF_NAME#v}"

README.md

@@ -136,7 +136,8 @@ FRBIT_DASHBOARD_URL=http://localhost:3001 \
 link during interactive login.
 
 Host resolution is: `--host`, `FRBIT_HOST`, saved host, then the production
-default.
+default. The CLI warns before it sends credentials to a non-default host; only
+use an override you trust.
 
 ## Release
 
@@ -149,7 +150,12 @@ just release --major
 
 CI validates formatting, static analysis, tests, builds, and the GoReleaser
 configuration. Pushing a `v*` tag creates the GitHub release archives, checksums,
-SBOMs, Homebrew formula, and npm packages.
+SBOMs, Homebrew formula, and npm packages. The release job runs in the
+review-gated `release` environment and publishes a GitHub build-provenance
+attestation for `checksums.txt`. When the GitHub CLI (`gh`) is installed, the
+installer verifies that attestation before installing. To require this check,
+set `FRBIT_VERIFY_PROVENANCE=1`; otherwise the installer warns and continues
+when `gh` is unavailable.
 
 ## License
 

SECURITY.md

@@ -0,0 +1,11 @@
+# Security Policy
+
+## Reporting a vulnerability
+
+Please do not open a public issue for a suspected vulnerability. Email
+security@fortrabbit.com with a description, reproduction steps, and any
+relevant impact. We will acknowledge reports and coordinate a fix privately.
+
+## Supported versions
+
+Security fixes are applied to the latest released version of `frbit`.

docs/cli.md

@@ -236,7 +236,8 @@ frbit environments update en-a1b2c3 --clear-build-commands
 Read or merge environment variables. `--set` and `--delete` are repeatable:
 
 ```sh
-frbit environments variables get en-a1b2c3
+frbit environments variables get en-a1b2c3             # values are masked
+frbit environments variables get en-a1b2c3 --reveal    # show values explicitly
 frbit environments variables update en-a1b2c3 \
   --set APP_ENV=production \
   --delete OLD_FLAG

install.sh

@@ -48,6 +48,16 @@ else
   release_base_url="https://github.com/$repository/releases/latest/download"
 fi
 
+validate_release_base_url() {
+  case "$release_base_url" in
+    https://github.com/fortrabbit/frbit-cli/releases/download/*|https://github.com/fortrabbit/frbit-cli/releases/latest/download) ;;
+    file://*) [ "${FRBIT_INSTALL_TESTING:-}" = "1" ] || fail "FRBIT_RELEASE_BASE_URL must use the official GitHub release host" ;;
+    *) fail "FRBIT_RELEASE_BASE_URL must use the official GitHub release host" ;;
+  esac
+}
+
+validate_release_base_url
+
 if [ -n "${FRBIT_INSTALL_DIR:-}" ]; then
   install_dir=${FRBIT_INSTALL_DIR%/}
 elif [ "$(id -u)" -eq 0 ]; then
@@ -76,6 +86,23 @@ fi
 
 [ "$actual" = "$expected" ] || fail "checksum verification failed for $asset"
 
+verify_provenance() {
+  if command -v gh >/dev/null 2>&1; then
+    gh attestation verify "$tmp_dir/checksums.txt" \
+      --repo "$repository" \
+      --signer-workflow "$repository/.github/workflows/release.yml" \
+      --predicate-type "https://slsa.dev/provenance/v1" \
+      >/dev/null || fail "release provenance verification failed"
+    return
+  fi
+  if [ "${FRBIT_VERIFY_PROVENANCE:-}" = "1" ]; then
+    fail "GitHub CLI (gh) is required when FRBIT_VERIFY_PROVENANCE=1"
+  fi
+  printf 'frbit installer: GitHub CLI (gh) not found; skipping release provenance verification. Set FRBIT_VERIFY_PROVENANCE=1 to require this check during installation.\n' >&2
+}
+
+verify_provenance
+
 tar -xzf "$tmp_dir/$asset" -C "$tmp_dir" frbit
 mkdir -p "$install_dir"
 install -m 0755 "$tmp_dir/frbit" "$install_dir/frbit"

internal/agentskills/source.go

@@ -152,7 +152,7 @@ func (s *Service) fetchPayload(ctx context.Context, release Release) (payload, e
 			result.copilot, result.hasCopilot = file, true
 		case strings.HasPrefix(name, "skills/"):
 			parts := strings.SplitN(strings.TrimPrefix(name, "skills/"), "/", 2)
-			if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
+			if len(parts) != 2 || !validSkillName(parts[0]) || parts[1] == "" {
 				continue
 			}
 			if result.skills[parts[0]] == nil {
@@ -215,3 +215,19 @@ func wantedArchivePath(name string) bool {
 	return name == "VERSION" || name == "update.sh" || name == "uninstall.sh" ||
 		name == ".github/instructions/fortrabbit.instructions.md" || strings.HasPrefix(name, "skills/")
 }
+
+// validSkillName deliberately uses a platform-independent allowlist. Archive
+// paths always use slashes, but a backslash becomes a path separator on Windows.
+func validSkillName(name string) bool {
+	if name == "" || name == "." || name == ".." {
+		return false
+	}
+	for _, character := range name {
+		if (character >= 'a' && character <= 'z') || (character >= 'A' && character <= 'Z') ||
+			(character >= '0' && character <= '9') || strings.ContainsRune("._-", character) {
+			continue
+		}
+		return false
+	}
+	return true
+}

internal/agentskills/types_test.go

@@ -89,3 +89,16 @@ func TestCopilotRequiresProjectScope(t *testing.T) {
 		t.Fatal("expected project-scope error")
 	}
 }
+
+func TestValidSkillNameRejectsPlatformPathSeparators(t *testing.T) {
+	for _, name := range []string{"", ".", "..", "../outside", `..\\outside`, `skill\\name`, "skill/name"} {
+		if validSkillName(name) {
+			t.Errorf("validSkillName(%q) = true, want false", name)
+		}
+	}
+	for _, name := range []string{"fortrabbit", "fortrabbit-api-access", "skill_2.0"} {
+		if !validSkillName(name) {
+			t.Errorf("validSkillName(%q) = false, want true", name)
+		}
+	}
+}

internal/app/factory.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"os"
 	"strings"
+	"time"
 
 	"github.com/fortrabbit/frbit-cli/internal/browser"
 	"github.com/fortrabbit/frbit-cli/internal/config"
@@ -35,7 +36,9 @@ func NewFactory(version string, commit string, date string) *Factory {
 		panic(fmt.Sprintf("initialize config store: %v", err))
 	}
 
-	httpClient := &http.Client{}
+	// This client is also used by skills downloads, which do not pass through
+	// api.NewClient's per-origin timeout wrapper.
+	httpClient := &http.Client{Timeout: 30 * time.Second}
 	checker := update.NewChecker(httpClient)
 
 	return &Factory{

internal/app/factory_test.go

@@ -0,0 +1,12 @@
+package app
+
+import (
+	"testing"
+	"time"
+)
+
+func TestNewFactorySetsHTTPTimeout(t *testing.T) {
+	if got := NewFactory("test", "test", "test").HTTPClient.Timeout; got != 30*time.Second {
+		t.Fatalf("HTTP timeout = %s, want %s", got, 30*time.Second)
+	}
+}

internal/cmd/environments/environments.go

@@ -1,6 +1,7 @@
 package environments
 
 import (
+	"encoding/json"
 	"fmt"
 	"io"
 	"text/tabwriter"
@@ -256,6 +257,7 @@ func newCmdVariables(factory *app.Factory) *cobra.Command {
 
 func newCmdVariablesGet(factory *app.Factory) *cobra.Command {
 	var printJSON bool
+	var reveal bool
 	command := &cobra.Command{
 		Use:   "get <public-id>",
 		Short: "Get custom and platform environment variables",
@@ -269,14 +271,18 @@ func newCmdVariablesGet(factory *app.Factory) *cobra.Command {
 			if err != nil {
 				return err
 			}
-			if printJSON {
+			if printJSON && reveal {
 				_, err = fmt.Fprintf(cmd.OutOrStdout(), "%s\n", response.Raw)
 				return err
 			}
-			return writeEnvironmentVariables(cmd.OutOrStdout(), response.Resource)
+			if printJSON {
+				return writeEnvironmentVariablesJSON(cmd.OutOrStdout(), response.Resource)
+			}
+			return writeEnvironmentVariables(cmd.OutOrStdout(), response.Resource, reveal)
 		},
 	}
-	command.Flags().BoolVar(&printJSON, "json", false, "Print the API response as JSON")
+	command.Flags().BoolVar(&printJSON, "json", false, "Print environment variables as JSON (values remain masked unless --reveal is set)")
+	command.Flags().BoolVar(&reveal, "reveal", false, "Show environment variable values")
 	return command
 }
 
@@ -327,11 +333,11 @@ func newCmdVariablesUpdate(factory *app.Factory) *cobra.Command {
 				_, err = fmt.Fprintf(cmd.OutOrStdout(), "%s\n", response.Raw)
 				return err
 			}
-			return writeEnvironmentVariables(cmd.OutOrStdout(), response.Resource)
+			return writeEnvironmentVariables(cmd.OutOrStdout(), response.Resource, false)
 		},
 	}
 	command.Flags().StringVarP(&file, "file", "f", "", "Read the complete request body from a JSON file ('-' for stdin)")
-	command.Flags().StringArrayVar(&setValues, "set", nil, "Variable to create or update as NAME=VALUE (repeatable)")
+	command.Flags().StringArrayVar(&setValues, "set", nil, "Variable to create or update as NAME=VALUE (repeatable; use --file or --file - for secrets)")
 	command.Flags().StringArrayVar(&deleteNames, "delete", nil, "Variable name to delete (repeatable)")
 	command.Flags().BoolVar(&printJSON, "json", false, "Print the API response as JSON")
 	return command
@@ -386,7 +392,7 @@ func writeResourceResponse(command *cobra.Command, response api.ResourceResponse
 	return resource.WriteResource(command.OutOrStdout(), response.Resource)
 }
 
-func writeEnvironmentVariables(output io.Writer, value api.Resource) error {
+func writeEnvironmentVariables(output io.Writer, value api.Resource, reveal bool) error {
 	table := tabwriter.NewWriter(output, 0, 4, 2, ' ', 0)
 	wroteHeader := false
 	for _, kind := range []string{"custom", "platform"} {
@@ -402,8 +408,8 @@ func writeEnvironmentVariables(output io.Writer, value api.Resource) error {
 				}
 				wroteHeader = true
 			}
-			variableValue := ""
-			if variable["value"] != nil {
+			variableValue := "***"
+			if reveal && variable["value"] != nil {
 				variableValue = fmt.Sprint(variable["value"])
 			}
 			if _, err := fmt.Fprintf(table, "%s\t%s\t%s\n", kind, fmt.Sprint(variable["name"]), variableValue); err != nil {
@@ -417,3 +423,37 @@ func writeEnvironmentVariables(output io.Writer, value api.Resource) error {
 	}
 	return table.Flush()
 }
+
+func writeEnvironmentVariablesJSON(output io.Writer, value api.Resource) error {
+	masked := make(api.Resource, len(value))
+	for key, raw := range value {
+		variables, ok := raw.([]any)
+		if !ok || (key != "custom" && key != "platform") {
+			masked[key] = raw
+			continue
+		}
+		entries := make([]any, len(variables))
+		for index, entry := range variables {
+			variable, ok := entry.(map[string]any)
+			if !ok {
+				entries[index] = entry
+				continue
+			}
+			copy := make(map[string]any, len(variable))
+			for field, fieldValue := range variable {
+				copy[field] = fieldValue
+			}
+			if copy["value"] != nil {
+				copy["value"] = "***"
+			}
+			entries[index] = copy
+		}
+		masked[key] = entries
+	}
+	encoded, err := json.Marshal(masked)
+	if err != nil {
+		return fmt.Errorf("encode environment variables: %w", err)
+	}
+	_, err = fmt.Fprintf(output, "%s\n", encoded)
+	return err
+}

internal/cmd/environments/environments_test.go

@@ -0,0 +1,38 @@
+package environments
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"github.com/fortrabbit/frbit-cli/internal/api"
+)
+
+func TestWriteEnvironmentVariablesMasksValuesUnlessRevealed(t *testing.T) {
+	variables := api.Resource{"custom": []any{map[string]any{"name": "SECRET", "value": "not-for-output"}}}
+	masked := &bytes.Buffer{}
+	if err := writeEnvironmentVariables(masked, variables, false); err != nil {
+		t.Fatal(err)
+	}
+	if got := masked.String(); strings.Contains(got, "not-for-output") || !strings.Contains(got, "***") {
+		t.Fatalf("masked output = %q", got)
+	}
+
+	revealed := &bytes.Buffer{}
+	if err := writeEnvironmentVariables(revealed, variables, true); err != nil {
+		t.Fatal(err)
+	}
+	if got := revealed.String(); !strings.Contains(got, "not-for-output") {
+		t.Fatalf("revealed output = %q", got)
+	}
+}
+
+func TestWriteEnvironmentVariablesJSONMasksValues(t *testing.T) {
+	output := &bytes.Buffer{}
+	if err := writeEnvironmentVariablesJSON(output, api.Resource{"custom": []any{map[string]any{"name": "SECRET", "value": "not-for-output"}}}); err != nil {
+		t.Fatal(err)
+	}
+	if got := output.String(); strings.Contains(got, "not-for-output") || !strings.Contains(got, `"value":"***"`) {
+		t.Fatalf("masked JSON = %q", got)
+	}
+}

internal/cmd/resource/resource.go

@@ -9,6 +9,7 @@ import (
 	"sort"
 	"strings"
 	"text/tabwriter"
+	"unicode"
 
 	"github.com/fortrabbit/frbit-cli/internal/api"
 	"github.com/fortrabbit/frbit-cli/internal/app"
@@ -310,13 +311,22 @@ func writeLogs(output io.Writer, resource api.Resource) error {
 		if !ok {
 			continue
 		}
-		if _, err := fmt.Fprintf(output, "%s\t%s\n", resourceValue(entry["time"]), resourceValue(entry["log"])); err != nil {
+		if _, err := fmt.Fprintf(output, "%s\t%s\n", resourceValue(entry["time"]), stripControlCharacters(resourceValue(entry["log"]))); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
+func stripControlCharacters(value string) string {
+	return strings.Map(func(character rune) rune {
+		if unicode.IsControl(character) {
+			return -1
+		}
+		return character
+	}, value)
+}
+
 func resourceValue(value any) string {
 	if value == nil {
 		return ""