view: toggle-first config UI + stable --token-file - #178
Conversation
Reviewer's GuideReworks Sequence diagram for immediate config toggle and syncsequenceDiagram
participant User
participant UI as ConfigUI
participant API as ConfigAPI
participant YAML as CanonicalYAML
participant Agents
User->>UI: Toggle setting
UI->>API: PATCH /api/config
API->>YAML: Apply operation with expected_revision
YAML-->>API: New revision
API-->>UI: revision
UI-->>User: Toggle applied, sync required
User->>UI: Click Sync now
UI->>API: POST /api/sync/preview
API-->>UI: Sync plan
alt Destructive changes
UI-->>User: Request confirmation
User->>UI: Confirm sync
end
UI->>API: POST /api/sync/apply
API->>Agents: Materialize configuration
Agents-->>API: Sync complete
API-->>UI: Success
Sequence diagram for stable view session tokensequenceDiagram
participant User
participant CLI as dotagentsView
participant Token as TokenFile
participant Server as ConfigWebServer
User->>CLI: dotagents view --token-file PATH
CLI->>Token: Read PATH
alt Existing non-empty token
Token-->>CLI: Stable token
else Missing token
CLI->>CLI: randomToken
CLI->>Token: Write token with mode 0600
Token-->>CLI: Token persisted
end
CLI->>Server: Start with session token
Server-->>User: Bookmarkable access URL
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="cmd/dotagents/config_web.go" line_range="124-127" />
<code_context>
+ if tokenFile == "" {
+ return randomToken(32)
+ }
+ switch data, err := os.ReadFile(tokenFile); {
+ case err == nil:
+ if tok := strings.TrimSpace(string(data)); tok != "" {
+ return tok, nil
+ }
+ case !errors.Is(err, os.ErrNotExist):
+ return "", fmt.Errorf("read token file %s: %w", tokenFile, err)
+ }
+ tok, err := randomToken(32)
+ if err != nil {
+ return "", err
+ }
+ if err := os.WriteFile(tokenFile, []byte(tok+"\n"), 0o600); err != nil {
+ return "", fmt.Errorf("write token file %s: %w", tokenFile, err)
+ }
</code_context>
<issue_to_address>
**🚨 issue (security):** When `tokenFile` already exists but is empty, `resolveServerToken` mints a token through `os.WriteFile` without changing the existing file mode, so an existing permissive mode such as `0644` remains in place despite the first-use token being documented as `0600`.
**Triggers:** When the configured token path is pre-created by deployment tooling or a previous failed initialization with permissions broader than `0600`.
**Suggested fix:** Open or rewrite the token file with explicit restrictive permissions and call `os.Chmod(tokenFile, 0o600)` after validating the file is the intended token file.
```suggestion
if err := os.WriteFile(tokenFile, []byte(tok+"\n"), 0o600); err != nil {
return "", fmt.Errorf("write token file %s: %w", tokenFile, err)
}
if err := os.Chmod(tokenFile, 0o600); err != nil {
return "", fmt.Errorf("chmod token file %s: %w", tokenFile, err)
}
return tok, nil
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and the opt-in token file changes the lifetime of the bearer credential protecting the web UI; if token persistence or file handling is wrong, previously distributed URLs may continue granting access after a revert and cannot be revoked by rerunning the old code. The UI also now persists configuration on every toggle, while sync can propagate those changes to agents, although destructive sync still requires confirmation.
Blocking findings: cmd/dotagents/config_web.go:127
| if err := os.WriteFile(tokenFile, []byte(tok+"\n"), 0o600); err != nil { | ||
| return "", fmt.Errorf("write token file %s: %w", tokenFile, err) | ||
| } | ||
| return tok, nil |
There was a problem hiding this comment.
🚨 issue (security): When tokenFile already exists but is empty, resolveServerToken mints a token through os.WriteFile without changing the existing file mode, so an existing permissive mode such as 0644 remains in place despite the first-use token being documented as 0600.
Triggers: When the configured token path is pre-created by deployment tooling or a previous failed initialization with permissions broader than 0600.
Suggested fix: Open or rewrite the token file with explicit restrictive permissions and call os.Chmod(tokenFile, 0o600) after validating the file is the intended token file.
| if err := os.WriteFile(tokenFile, []byte(tok+"\n"), 0o600); err != nil { | |
| return "", fmt.Errorf("write token file %s: %w", tokenFile, err) | |
| } | |
| return tok, nil | |
| if err := os.WriteFile(tokenFile, []byte(tok+"\n"), 0o600); err != nil { | |
| return "", fmt.Errorf("write token file %s: %w", tokenFile, err) | |
| } | |
| if err := os.Chmod(tokenFile, 0o600); err != nil { | |
| return "", fmt.Errorf("chmod token file %s: %w", tokenFile, err) | |
| } | |
| return tok, nil |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33360acbb8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case err == nil: | ||
| if tok := strings.TrimSpace(string(data)); tok != "" { | ||
| return tok, nil |
There was a problem hiding this comment.
On a multi-user host where an existing token file is group/world-readable, this branch returns its bearer token without checking or tightening its mode. Because that token can be exchanged for a session that invokes config mutation and sync APIs over loopback, another local user with access to the path can take control of the service; reject insecure/non-regular files or chmod them to 0600 before reuse.
Useful? React with 👍 / 👎.
| configuration is read-only. In `view`, each toggle applies immediately; neither | ||
| surface runs `sync` implicitly. `view` binds only to loopback and uses a session cookie plus |
There was a problem hiding this comment.
This updates only the skill prose for immediate toggle writes: README.md:122-140 still promises a review-first save and preview/confirm workflow, while both README.md:89 and the command list at skills/dotagents/SKILL.md:28 omit the new --token-file option. Update those surfaces and the release-site copy so users are not given the obsolete workflow and can discover the persistent-service flag.
AGENTS.md reference: AGENTS.md:L51-L51
Useful? React with 👍 / 👎.
| input.disabled = true; | ||
| try { | ||
| const result = await api('/api/config/validate', {method:'POST', body:JSON.stringify({layer, operations:[...pendingOperations.values()]})}); | ||
| $('#diff').textContent = result.diff || '(no changes)'; | ||
| setStatus('Change staged. Review the diff, then save.', 'ok'); | ||
| const result = await api('/api/config', {method:'PATCH', body:JSON.stringify({layer, expected_revision:state.revision, operations:[{op:'set', path, value}]})}); | ||
| state.revision = result.revision; |
There was a problem hiding this comment.
When a user flips a second row before the first PATCH returns, both requests carry the same state.revision because only the clicked input is disabled. After one request advances the revision, every other in-flight toggle receives stale_revision and is reloaded away, so quickly enabling several agents, hooks, or servers applies only one of the requested changes; queue PATCHes or disable all editable controls until the revision is updated.
Useful? React with 👍 / 👎.
1 participant
Reworks the canonical config web UI (
dotagents view) to be toggle-first, and adds a stable session token for persistent hosting.UI
PATCH /api/configon the canonical write path (revision-guarded), instead of staging into a Validate → Review-diff → Save change-rail.ui.links(e.g. an "AI usage" link). Effective layer stays read-only.Server
--token-file PATHfordotagents view: a stable session token (minted0600on first use) so a restarted persistent service keeps one bookmarkable URL. Without it, behavior is unchanged (fresh per-process token).Tests / verify
config_web_token_test.go: ephemeral-by-default, stable-from-file (0600), flag parse. Existing single-opPATCHcontract already covered.AI usage→/usage.go test ./...+go vetgreen.Note: PR #177's demo screenshots show the old change-rail/Save UI removed here and will need reshooting; mind merge order.
Summary by Sourcery
Streamline
dotagents viewaround immediate configuration toggles and add optional stable session URLs for persistent hosting.New Features:
dotagents view, preserving its access URL across service restarts.Enhancements:
Documentation:
Tests: