Skip to content

docs: add Station profiles security roadmap#346

Open
zozo123 wants to merge 4 commits into
openclaw:mainfrom
zozo123:docs/station-profiles-security-roadmap
Open

docs: add Station profiles security roadmap#346
zozo123 wants to merge 4 commits into
openclaw:mainfrom
zozo123:docs/station-profiles-security-roadmap

Conversation

@zozo123

@zozo123 zozo123 commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Test plan

  • node scripts/check-docs-links.mjs
  • node scripts/check-command-docs.mjs

Refs #193.

Made with Cursor

@clawsweeper

clawsweeper Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 14, 2026, 4:37 AM ET / 08:37 UTC.

Summary
The PR adds a Station profiles roadmap and security-doc links, plus an unwired internal/station Go package for StationProfile parsing, phase gates, AgentProfile, ModelAccessPolicy, and tests.

Reproducibility: not applicable. as a user bug report. The PR review finding is source-reproducible by inspecting AuthorizeModelAccess against the current env.allow wildcard/name contract.

Review metrics: 2 noteworthy metrics.

  • Touched files: 8 files changed: 5 Go files added, 3 docs files changed. The branch is now runtime/test skeleton work plus docs, so docs-only review and proof expectations no longer apply.
  • Added surface: 944 additions, 0 deletions. A large additive product/security surface needs maintainer sign-off even though no station command is wired yet.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted terminal or live output for the relevant Go tests and docs checks after the current head.
  • Remove the internal/station skeleton from this docs PR, or harden the modelAccess/env.allow boundary with focused tests.
  • Get maintainer/security sign-off before landing any Station/modelAccess runtime API names or invariants.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Missing: this external PR adds non-docs Go code but only lists docs link checks; add redacted terminal/live output for the relevant Go and docs checks, update the PR body for a fresh review, and redact private values such as API keys, IPs, phone numbers, and non-public endpoints.

Risk before merge

  • [P1] Merging would publish and partially codify Station/modelAccess as repository direction while the linked security/product roadmap remains open.
  • [P1] The new AuthorizeModelAccess check can approve modelAccess while credential-like env.allow entries or wildcard patterns remain forwarded through the ordinary environment path.
  • [P1] The PR lacks real behavior proof for the added Go package and current validation, so contributor-run behavior has not been shown after the latest head.
  • [P1] The Station evidence plan depends on harness/proof work whose previous broad PR was closed unmerged, leaving the reuse contract unsettled.

Maintainer options:

  1. Split implementation from roadmap (recommended)
    Before merge, remove internal/station from this PR and keep only the docs that maintainers want as roadmap context.
  2. Harden the credential boundary
    If maintainers want the skeleton now, replace the gateway-name equality check with a structurally safe modelAccess/env.allow separation and focused wildcard/credential-name tests.
  3. Accept as explicit skeleton
    Maintainers can intentionally accept the unwired package only after product/security sign-off that these API names and invariants are approved direction.

Next step before merge

  • [P1] Maintainers need to decide whether to merge any Station/modelAccess skeleton now and how the credential boundary should be designed; this is not a narrow automation repair.

Security
Needs attention: Needs attention: the new modelAccess gate can give a false sense of separation from env.allow credential forwarding.

Review findings

  • [P1] Do not use gateway names as the env.allow credential boundary — internal/station/gate.go:124-126
Review details

Best possible solution:

Keep any maintainer-approved roadmap docs, but remove or defer internal/station until Phase 1 Station is explicitly approved without model credentials and modelAccess has a structurally safe credential boundary.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a user bug report. The PR review finding is source-reproducible by inspecting AuthorizeModelAccess against the current env.allow wildcard/name contract.

Is this the best way to solve the issue?

No: a docs roadmap may be reasonable, but merging an unwired modelAccess gate that only checks the gateway name against env.allow is not the narrowest safe solution. Split docs from implementation or harden/remove the gate after maintainer and security approval.

Full review comments:

  • [P1] Do not use gateway names as the env.allow credential boundary — internal/station/gate.go:124-126
    AuthorizeModelAccess claims model/tool credentials never travel through ordinary env.allow, but it rejects only entries exactly equal to ModelAccess.Gateway. Existing env forwarding is name-based and supports trailing wildcard prefixes, so credential names such as OPENAI_API_KEY, ANTHROPIC_API_KEY, or MODEL_* can still be allowed while modelAccess is authorized; make the separation structural or remove this gate until the design is approved.
    Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against ccc27374948c.

Label changes

Label justifications:

  • P2: This is a normal-priority but security-sensitive feature skeleton with a concrete modelAccess boundary blocker.
  • merge-risk: 🚨 security-boundary: The PR adds a modelAccess credential gate whose env.allow separation check is too narrow for the documented secret-forwarding model.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Missing: this external PR adds non-docs Go code but only lists docs link checks; add redacted terminal/live output for the relevant Go and docs checks, update the PR body for a fresh review, and redact private values such as API keys, IPs, phone numbers, and non-public endpoints.
Evidence reviewed

Security concerns:

  • [medium] modelAccess gate does not prove env.allow separation — internal/station/gate.go:124
    The gate only compares envAllow entries to the configured gateway name, while Crabbox env.allow supports arbitrary names and trailing wildcards; credential-like env names can still be forwarded while modelAccess is authorized.
    Confidence: 0.88

What I checked:

  • Repository policy read: AGENTS.md was read fully; its generic product-positioning and security/configuration guidance applies because this PR adds Station/modelAccess roadmap text and credential-boundary code. (AGENTS.md:1, ccc27374948c)
  • PR scope changed beyond docs: GitHub PR metadata shows 8 changed files with 944 additions and 0 deletions: 3 docs files plus 5 added Go files under internal/station. (09be5ffb3bbc)
  • Current main has no Station/modelAccess surface: Current main search found no Station, stationProfile, modelAccess, PhaseModelAccess, or internal/station implementation, so the PR is not obsolete on main. (ccc27374948c)
  • modelAccess env.allow gate is too narrow: The new authorization loop rejects only env.allow entries exactly equal to p.ModelAccess.Gateway, while the comment claims it asserts any overlap between env.allow names and model-access intent. (internal/station/gate.go:124, 09be5ffb3bbc)
  • Current env.allow contract supports wildcard names: Current docs say env.allow is name-based and supports trailing wildcards, so credential-shaped names such as OPENAI_API_KEY or wildcard patterns can still be forwarded without matching the gateway string. (docs/features/configuration.md:893, ccc27374948c)
  • Linked roadmap remains open and security-sensitive: The linked roadmap remains open with security/auth labels, and its recent discussion recommends keeping Station/modelAccess as product/security roadmap work rather than starting broad implementation before Phase 1 is split.

Likely related people:

  • Peter Steinberger: Current blame for env/security docs and recent main history for workspace/runtime lifecycle files point to Peter's commits in the surfaces this PR extends. (role: recent area contributor; confidence: high; commits: 9e208c80cd1a, 7763eecdf759, 81ef2a83be56; files: docs/security.md, docs/features/configuration.md, internal/cli/run_env_profile.go)
  • zozo123: This handle authored the linked roadmap and has current-main history in delegated provider, pause/resume, and evidence-artifact areas that are adjacent to Station lifecycle/proof design. (role: roadmap proposer and adjacent area contributor; confidence: medium; commits: 8b246f6f96b7, b2356e0f5173, f50fa9691968; files: internal/cli/provider_backend.go, internal/cli/config.go, docs/features/README.md)
  • vincentkoc: Vincent closed the related harness dependency with a product-design rationale, and local history shows security/provider hardening work near the credential and provider boundary concerns. (role: related reviewer and adjacent security contributor; confidence: medium; commits: 0a76d7a8bbed, 8ed6588b2e40, e7be0273d063; files: internal/cli/config.go, internal/cli/provider_backend.go, docs/security.md)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Document the Station profile and modelAccess boundaries from issue openclaw#193 so future implementation PRs have reviewable phase gates.

Co-authored-by: Cursor <cursoragent@cursor.com>
@zozo123 zozo123 force-pushed the docs/station-profiles-security-roadmap branch from 30ebfc8 to 27c0d91 Compare June 14, 2026 06:58
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels Jun 14, 2026
zozo123 and others added 2 commits June 14, 2026 11:03
Scaffold the first buildable slice of the Station profile primitive from
issue openclaw#193, matching the disabled-by-default skeleton style used elsewhere
in the repo.

internal/station provides:
- StationProfile config struct with YAML parsing and validation
- the AgentProfile boundary type (repo-owned command, Crabbox-supervised)
- feature-gated phase enforcement (Gate) that returns clear
  "not yet enabled" errors until each roadmap phase is turned on
- ModelAccessPolicy as a separate, audited field that is never sourced
  from env.allow, with AuthorizeModelAccess rejecting any gateway leaked
  through env.allow forwarding

Nothing is enabled by default: profiles are inert unless explicitly
enabled, and no station command, lifecycle, or live credential delivery
is wired up yet (those land in later, separately reviewed phases).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Update the Station profiles roadmap status to reflect that
internal/station now ships the disabled-by-default config primitive,
agent-profile boundary, phase gates, and env.allow-separated modelAccess
gating, while the CLI command and live credential delivery remain future
phases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels Jun 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant