Skip to content

fix: harden P0 security controls and Build response header timeout (#731)#735

Closed
Avilianb wants to merge 2 commits into
chenyme:mainfrom
Avilianb:fix/security-p0-and-731
Closed

fix: harden P0 security controls and Build response header timeout (#731)#735
Avilianb wants to merge 2 commits into
chenyme:mainfrom
Avilianb:fix/security-p0-and-731

Conversation

@Avilianb

@Avilianb Avilianb commented Jul 21, 2026

Copy link
Copy Markdown

Summary

Addresses high-priority security findings from a static audit of grok2api, plus open issue #731.

Security (P0)

  1. Provider BaseURL host allowlistprovider.build/web/console.baseURL must be x.ai or grok.com (or subdomains). Prevents a compromised admin session from redirecting credential-bearing egress to an attacker host.
  2. Statsig signer internal URLs (file-auth model) — Internal/private signer URLs require provider.web.allowInternalStatsigSigner: true in the YAML config file only. The flag is never set from admin hot settings. When the flag is true, runtime settings may set/change internal signer URLs (Docker/K8s service names). When false, Validate rejects all internal signer URLs (SSRF control).
    • applyDomainConfig / LoadPersisted / ReloadPersisted preserve the file-only flag so reloads do not wipe Docker/K8s setups.
    • Hot updates do not clear the flag when the signer URL changes (flag is file-owned).
  3. Media hard capacitySaveImage / video commit reject before write when total + size > MaxTotalBytes (ErrMediaQuotaExceeded).
  4. Media Cache-Control — Public capability media GET uses private, max-age=3600 instead of year-long public, immutable.
  5. Secure cookies — When effective publicApiBaseURL is HTTPS, auth.secureCookies must be true (config validation).

Bugfix

  1. [Bug]: Grok Build 请求等待响应头 30 秒即超时,返回 502 与 http2: timeout awaiting response headers #731 — Build HTTP client ResponseHeaderTimeout raised from 30s → 10m in both egress newBuildClient and CLI adapter fallback transport. Large Build payloads no longer fail with http2: timeout awaiting response headers while waiting for the first response header. Body streaming remains governed by the request context / server request timeout.

Follow-up hardening (this push)

  • Fix regression where applyDomainConfig rebuilt WebProviderConfig without AllowInternalStatsigSigner, breaking YAML-allowed internal signers after DB load/reload.
  • Map ErrMediaQuotaExceededHTTP 507 on public video PUT.
  • Document provider.web.allowInternalStatsigSigner in config.example.yaml with accurate usage (file-only; enables admin-set internal signer when true).
  • Integration-style tests: load/reload preserve flag; hot-update internal signer allowed only with flag; 507 on quota.

Test plan

  • go test on egress, signerurl, config, cli, settings (incl. allowInternal load/reload/hot-update), media (SaveImage/quota), media handler (Cache-Control, 507)
  • Deploy with HTTPS publicApiBaseURL and confirm secureCookies: true is required
  • Attempt hot-update of BaseURL to non-*.x.ai/*.grok.com host → rejected
  • YAML allowInternalStatsigSigner: true + admin set http://grok-signer-go:8788/sign → accepted; without flag → rejected
  • Large Build prompt that previously hit 30s header timeout → succeeds or fails only on real upstream errors
  • Fill media store to MaxTotalBytes → further saves/uploads return quota error (507 on video PUT)

Notes

  • Capability URL media model (unguessable IDs, no API key on GET) is unchanged; only cache policy and capacity enforcement tightened.
  • Correct usage for Docker/K8s internal Statsig signer: set provider.web.allowInternalStatsigSigner: true in the config file, then set the signer URL in admin runtime settings (or keep the default public signer). The admin UI cannot enable the allow flag itself.
  • Windows-local video sync tests may fail with Access is denied independently of this change (observed on clean upstream/main as well).

Review status

Prior review: request changes on Statsig path (flag cleared by applyDomainConfig / hot-update clear conflicted with PR claim). Those merge conditions are addressed in e9af54f.

woo added 2 commits July 22, 2026 03:23
- Whitelist provider BaseURL hosts to x.ai / grok.com (blocks admin credential exfil)
- Require allowInternalStatsigSigner for internal signer URLs; hot updates cannot introduce them
- Hard-reject media writes when MaxTotalBytes would be exceeded
- Use private Cache-Control for capability media URLs
- Require secureCookies when publicApiBaseURL is HTTPS
- Raise Build ResponseHeaderTimeout from 30s to 10m (fixes chenyme#731)
- Keep AllowInternalStatsigSigner from YAML in applyDomainConfig
- Stop clearing the flag on hot signer URL changes (file-auth model)
- Map ErrMediaQuotaExceeded to HTTP 507 on video upload
- Document allowInternalStatsigSigner in config.example.yaml
- Add load/reload/hot-update tests for internal Statsig signer policy
@chenyme

chenyme commented Jul 22, 2026

Copy link
Copy Markdown
Owner
  1. BaseURL 硬编码域名限制不合理,这可能是用户自定义的 Grok 镜像地址。
  2. 媒体硬容量存在并发和启动漏洞
  • 进程启动时容量计数从 0 开始,后台任务稍后才加载数据库占用,清理和删除流程会覆盖正在进行的容量预留。多实例各自维护计数,无法形成全局硬限制,因此仍可能突破 MaxTotalBytes。

@chenyme
chenyme marked this pull request as draft July 22, 2026 12:30
@chenyme chenyme closed this Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants