Skip to content

add pid file for gateway running and auth token for /reload and pico channel#2134

Merged
yinwm merged 6 commits intosipeed:mainfrom
cytown:t3
Mar 30, 2026
Merged

add pid file for gateway running and auth token for /reload and pico channel#2134
yinwm merged 6 commits intosipeed:mainfrom
cytown:t3

Conversation

@cytown
Copy link
Copy Markdown
Contributor

@cytown cytown commented Mar 28, 2026

📝 Description

Add PID file management and auth tokens for gateway lifecycle and pico channel

This commit introduces centralized PID file management and token-based authentication for the gateway process. Key changes:

New pkg/pid package — Cross-platform PID file management (JSON format with PID, token, version, port, host). Supports singleton enforcement, atomic writes (temp + rename), stale PID cleanup, and platform-specific process liveness checks (Unix: signal(0), Windows: OpenProcess/GetExitCodeProcess). Comprehensive test coverage included.

Gateway singleton & auth token — The gateway writes a PID file on startup with a cryptographically random auth token. This token is used to authenticate the /reload endpoint (subtle.ConstantTimeCompare) and compose the Pico channel WebSocket token (pico-).

WebSocket reverse proxy — The web backend now proxies /pico/ws to the gateway, allowing the frontend to connect through a single port. Token validation distinguishes between gateway unavailability (503) and invalid credentials (403).

Config centralization — Consolidated scattered home directory resolution into config.GetHome() with PICOCLAW_HOME env support. Extracted env var keys into config.EnvHome, config.EnvConfig, config.EnvGatewayHost etc. as typed constants.

TUI gateway management — Replaced inline PID file parsing and platform-specific process checking with the shared pkg/pid package. Uses SIGTERM instead of SIGKILL for graceful shutdown.

Middleware fix — Added Hijack() to responseRecorder so WebSocket upgrades work correctly through the logging middleware layer.

🗣️ Type of Change

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 📖 Documentation update
  • ⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

  • 🤖 Fully AI-generated (100% AI, 0% Human)
  • 🛠️ Mostly AI-generated (AI draft, Human verified/modified)
  • 👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

🔗 Related Issue

📚 Technical Context (Skip for Docs)

  • Reference URL:
  • Reasoning:

🧪 Test Environment

  • Hardware:
  • OS:
  • Model/Provider:
  • Channels:

📸 Evidence (Optional)

Click to view Logs/Screenshots

☑️ Checklist

  • My code/docs follow the style of this project.
  • I have performed a self-review of my own changes.
  • I have updated the documentation accordingly.

@sipeed-bot sipeed-bot bot added type: enhancement New feature or request domain: channel domain: config go Pull requests that update go code labels Mar 28, 2026
Copy link
Copy Markdown
Collaborator

@yinwm yinwm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! The PID file management design is solid and the config consolidation is a great cleanup. A few blocking issues before we can merge:


1. Security: Token leaked in logs (must fix)

In TryAutoStartGateway(), the line:

logger.Infof("pidData: %v", pidData)

will print the auth token to logs since %v dumps all struct fields including Token. Please either remove this line or use a format that excludes the token, e.g.:

logger.Infof("pidData: PID=%d, Version=%s", pidData.PID, pidData.Version)

2. Duplicated picoTokenPrefix constant (must fix)

picoTokenPrefix = "pico-" is defined in both pkg/gateway/gateway.go and web/backend/api/gateway.go. If one side changes and the other doesn't, token composition/validation will silently break. Please extract it to a shared location (e.g. pkg/pid or a dedicated constants file).


3. Breaking API changes not flagged (must fix)

This PR introduces breaking changes but is labeled as "non-breaking":

  • /health no longer returns the pid field
  • /reload now requires Bearer token authentication
  • /pico/ws now validates tokens before proxying
  • EnsurePicoChannel signature changed from error to (bool, error)

Please update the PR type checkbox accordingly and confirm the frontend is fully adapted to these changes.


Non-blocking suggestions:

  • web/backend/api/gateway.go is 870+ lines — consider splitting into gateway_pid.go / gateway_pico.go
  • picoComposedToken uses plain string comparison for token validation; consider subtle.ConstantTimeCompare for consistency with the /reload handler
  • gatewayStatusData() has 3 separate lock/unlock paths — worth simplifying the locking pattern
  • Unit test coverage for picoComposedToken, extractBearerToken, and the /reload auth flow would be valuable additions

Copy link
Copy Markdown
Collaborator

@yinwm yinwm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Great addition — the centralized PID file management and config.GetHome() consolidation clean up a lot of scattered logic. Token auth for /reload and the pico channel is well-designed with proper constant-time comparison.

A few non-blocking suggestions for future consideration:

  1. In overridePicoToken, the composite token pico-<pidToken><picoToken> lacks a delimiter — could cause parsing ambiguity downstream.
  2. generateToken's fallback to time.Now().UnixNano() is predictable — consider returning an error when the system CSPRNG is unavailable.
  3. Removing Pid from /health response is technically breaking — worth confirming no external consumers depend on it.
  4. refreshPicoTokensLocked silently swallows config load errors — a warning log would help debugging.
  5. gatewayStatusData has an attachToGatewayProcessLocked side effect despite its read-sounding name.

None of these block merging. Nice work!

@yinwm yinwm merged commit a5f8b0f into sipeed:main Mar 30, 2026
4 checks passed
@cytown cytown deleted the t3 branch April 1, 2026 05:28
ra1phdd pushed a commit to ra1phdd/picoclaw-pkg that referenced this pull request Apr 12, 2026
add pid file for gateway running and auth token for /reload and pico channel
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: channel domain: config go Pull requests that update go code type: enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants