security: Phase 2 high-priority fixes (H-1 through H-10)#39
Merged
initializ-mk merged 2 commits intomainfrom Mar 28, 2026
Merged
security: Phase 2 high-priority fixes (H-1 through H-10)#39initializ-mk merged 2 commits intomainfrom
initializ-mk merged 2 commits intomainfrom
Conversation
- H-1: Per-IP rate limiting middleware on A2A server (golang.org/x/time) - H-2: Request body size limits (2 MiB MaxBytesReader, 1 MiB MaxHeaderBytes) - H-3: Telegram webhook auth (secret token, loopback binding, content-type/size validation) - H-4: Slack event deduplication by envelope ID with TTL cache - H-5: npx --no-install flag for custom TypeScript tools - H-6: Trust policy defaults (RequireChecksum: true, unsigned skill warnings) - H-7: Symlink escape detection in skill scanner (ScanWithRoot) - H-8: Custom tool entrypoint validation (path traversal, symlink, absolute path) - H-10: Cross-category secret reuse detection at startup Additional fixes: - OAuth token resolution for skill scripts (SkillCommandExecutor) - Model passthrough via REVIEW_MODEL env var - GH_CONFIG_DIR preservation when HOME is overridden in cli_execute - cli_execute added to PII guardrail allow_tools - Code review scripts: Responses API streaming, markdown output format - Code review SKILL.md: added missing egress domains - Updated all affected documentation (7 docs)
Previously GH_CONFIG_DIR was set for all binaries when HOME was overridden, exposing GitHub credentials to every subprocess. Now only the gh binary receives this env var, limiting credential access to the binary that actually needs it.
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
1 participant
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
golang.org/x/timetoken bucket — read: 60 req/min burst 10, write: 10 req/min burst 3, auto-evict stale visitors)MaxBytesReader, 1 MiBMaxHeaderBytes, 413 on excess)npx --no-installflag for custom TypeScript tools to prevent automatic package downloadsRequireChecksum: true, unsigned skill warnings at scan time)ScanWithRootvalidates symlinks stay within root)ghbinary, not all subprocessesSkillCommandExecutorresolves__oauth__sentinel to real access token +OPENAI_BASE_URLREVIEW_MODELenv var for skill scriptscli_executeadded tono_piiallow_tools (notno_secrets)Test plan
cd forge-core && go test ./...cd forge-cli && go test ./...cd forge-plugins && go test ./...cd forge-skills && go test ./...Retry-Afterheader on burstnpx --no-installis first arg for TypeScript custom toolsGH_CONFIG_DIRonly appears in env forghbinary, not others