fix: harden P0 security controls and Build response header timeout (#731)#735
Closed
Avilianb wants to merge 2 commits into
Closed
fix: harden P0 security controls and Build response header timeout (#731)#735Avilianb wants to merge 2 commits into
Avilianb wants to merge 2 commits into
Conversation
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
Owner
|
chenyme
marked this pull request as draft
July 22, 2026 12:30
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses high-priority security findings from a static audit of grok2api, plus open issue #731.
Security (P0)
provider.build/web/console.baseURLmust bex.aiorgrok.com(or subdomains). Prevents a compromised admin session from redirecting credential-bearing egress to an attacker host.provider.web.allowInternalStatsigSigner: truein 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/ReloadPersistedpreserve the file-only flag so reloads do not wipe Docker/K8s setups.SaveImage/ video commit reject before write whentotal + size > MaxTotalBytes(ErrMediaQuotaExceeded).private, max-age=3600instead of year-longpublic, immutable.publicApiBaseURLis HTTPS,auth.secureCookiesmust betrue(config validation).Bugfix
ResponseHeaderTimeoutraised from 30s → 10m in both egressnewBuildClientand CLI adapter fallback transport. Large Build payloads no longer fail withhttp2: timeout awaiting response headerswhile waiting for the first response header. Body streaming remains governed by the request context / server request timeout.Follow-up hardening (this push)
applyDomainConfigrebuiltWebProviderConfigwithoutAllowInternalStatsigSigner, breaking YAML-allowed internal signers after DB load/reload.ErrMediaQuotaExceeded→ HTTP 507 on public video PUT.provider.web.allowInternalStatsigSignerinconfig.example.yamlwith accurate usage (file-only; enables admin-set internal signer when true).Test plan
go teston egress, signerurl, config, cli, settings (incl. allowInternal load/reload/hot-update), media (SaveImage/quota), media handler (Cache-Control, 507)publicApiBaseURLand confirmsecureCookies: trueis required*.x.ai/*.grok.comhost → rejectedallowInternalStatsigSigner: true+ admin sethttp://grok-signer-go:8788/sign→ accepted; without flag → rejectedMaxTotalBytes→ further saves/uploads return quota error (507 on video PUT)Notes
provider.web.allowInternalStatsigSigner: truein 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.Access is deniedindependently of this change (observed on cleanupstream/mainas 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 ine9af54f.