[security] fix: reject empty env allow wildcards

[Hinotoi-agent]

Summary

This PR hardens Crabbox's environment-variable forwarding boundary so a bare wildcard allow pattern cannot silently forward every local environment variable to a remote lease.

• Rejects env.allow: ['*'] / empty-prefix wildcard patterns in the env allowlist matcher.
• Preserves explicit exact matches such as CI and non-empty prefix wildcards such as PROJECT_*.
• Adds regression coverage for both the low-level matcher and the full repo-local .crabbox.yaml config path.

Security issues covered

Issue	Impact	Severity
Bare env.allow wildcard forwards all local environment variables	A malicious or compromised repo can use repo-local Crabbox config to forward local secrets such as API tokens, cloud credentials, and broker tokens into the remote command environment before the requested command runs.	Critical

Before this PR

• Crabbox automatically loaded repo-local crabbox.yaml / .crabbox.yaml.
• env.allow values ending in * were treated as prefix wildcards.
• The pattern * produced an empty prefix, and every environment-variable name has that prefix.
• allowedEnv() copied all matching values from os.Environ().
• Remote command construction then serialized those values into the SSH command prefix.

After this PR

• Empty-prefix wildcards are ignored instead of matching every variable.
• Existing explicit allow entries still work.
• Existing non-empty prefix wildcards still work.
• Tests cover both the matcher behavior and a repo-local .crabbox.yaml attempting env.allow: ['*'].

Why this matters

Crabbox's security docs describe environment forwarding as allowlisted and state that secrets should stay local unless explicitly allowed. Repo-local config is controlled by the repository being run, not necessarily by the operator. A repository-controlled env.allow: ['*'] therefore turns a convenience setting into a local secret exfiltration primitive.

Attack flow

malicious repository commits .crabbox.yaml with env.allow: ['*']
    -> operator runs crabbox from that repository
        -> Crabbox loads repo-local config and forwards all local env vars
            -> repo-controlled command can read/exfiltrate local secrets on the lease

Affected code

Issue	Files
Empty-prefix wildcard matches every env var	internal/cli/repo.go, internal/cli/config.go, internal/cli/run.go, internal/cli/ssh.go
Regression coverage	internal/cli/ssh_test.go, internal/cli/config_test.go

Root cause

Bare env.allow wildcard forwards all local environment variables:

• Direct cause: envAllowed() accepted any pattern ending in * and passed strings.TrimSuffix(pattern, "*") directly to strings.HasPrefix().
• Boundary failure: for *, the prefix is empty, so every local environment variable matched, including secret-looking values the operator did not individually allow.

CVSS assessment

Issue	CVSS v3.1	Vector
Repo-local bare env wildcard secret forwarding	9.1 Critical	CVSS:3.1/AV:L/AC:L/PR:N/UI:R/S:C/C:H/I:H/A:N

Rationale:

• The attack requires an operator to run Crabbox in a malicious or compromised repository, but no additional Crabbox privileges are required once that repo config is loaded.
• Scope changes because repo-controlled config can cause local operator secrets to be disclosed to a remote lease environment controlled by the run.
• Confidentiality impact is high because local cloud/API/broker tokens may be forwarded.
• Integrity impact is high when forwarded tokens grant write access to external systems.

Safe reproduction steps

1. In a Crabbox checkout before this fix, create a temporary repository directory containing:

   env:
     allow:
       - '*'

2. Set a proof environment variable, for example:

   export CRABBOX_PROOF_API_TOKEN=critical-secret-value

3. Load Crabbox config from inside that repository and inspect allowedEnv(cfg.EnvAllow).

4. Observe that the proof variable is included even though it was not individually allowlisted.

5. Build a remote command with remoteCommandWithEnvFile() and observe the proof variable serialized into the command prefix.

The regression test added in this PR performs the same check without contacting a remote provider.

Expected vulnerable behavior

• envAllowed("CRABBOX_PROOF_API_TOKEN", []string{"*"}) returned true.
• A repo-local .crabbox.yaml with env.allow: ['*'] caused allowedEnv(cfg.EnvAllow) to include the local proof token.
• The generated remote command could include the proof token assignment.

Changes in this PR

• Adds an empty-prefix guard to envAllowed() before applying prefix wildcard matching.
• Keeps non-empty prefix wildcard behavior unchanged.
• Adds matcher-level regression coverage for bare and whitespace-trimmed * patterns.
• Adds config-level regression coverage for repo-local .crabbox.yaml with env.allow: ['*'].

Files changed

Category	Files	What changed
Env allowlist hardening	internal/cli/repo.go	Ignore wildcard patterns whose prefix is empty.
Matcher regression tests	internal/cli/ssh_test.go	Verify bare * no longer matches all variables and PROJECT_* still works.
Repo config regression tests	internal/cli/config_test.go	Verify repo-local env.allow: ['*'] does not forward a proof secret.

Maintainer impact

• This is intentionally narrow: it only changes the pathological empty-prefix wildcard case.
• Existing exact allow entries continue to work.
• Existing non-empty prefix wildcard entries continue to work.
• No provider, Worker, or remote execution behavior changes beyond the environment values admitted by the allowlist.

Fix rationale

A wildcard matcher should never treat an empty prefix as authorization to forward every local environment variable. Requiring wildcard prefixes to be non-empty keeps the documented allowlist behavior useful while preventing the broadest repo-controlled secret forwarding case.

Type of change

• Security fix
• Tests
• Documentation update
• Refactor with no behavior change

Test plan

• Confirmed the new matcher regression fails on the vulnerable code path before the fix.
• Ran focused CLI regression tests.
• Ran the full Go test suite.
• Ran Go vet.
• Checked whitespace/diff hygiene.

Executed with:

go test ./internal/cli -run TestEnvAllowlistRejectsEmptyWildcardPrefix -count=1 -v
go test ./internal/cli -run 'TestEnvAllowlist|TestRepoConfigBareEnvWildcardDoesNotForwardEveryLocalVariable' -count=1 -v
go test ./internal/cli -count=1
go test ./... -count=1
go vet ./...
git diff --check

Token usage

• discovery tokens: partial/unknown
• validation tokens: partial/unknown
• duplicate-check tokens: partial/unknown
• PR/writeup tokens: partial/unknown
• total tokens: partial/unknown
• notes: exact token telemetry was not available in this local PR workflow.

Disclosure notes

• This PR is bounded to the bare wildcard environment-forwarding path.
• It does not claim to block every possible risky env.allow pattern; it removes the broad empty-prefix wildcard behavior while preserving explicit allowlist functionality.
• No unrelated files were changed.