mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-10-02 01:15:23 +08:00
Kamakhya Singhandkite
e333b0be4b
* security: validate credential commands and warn on plaintext secrets - Add validateKeyCmd() in internal/llm/keycmd.go to reject suspicious shell patterns (command separators, backticks, redirections) in api_key_cmd and auth_token_cmd before the shell executes them. Provides a clear diagnostic for tampered or accidentally malformed config rather than silently running destructive expressions. - Add plaintext credential warnings in internal/llm/resolver.go when api_key or auth_token are stored as static strings in the config file. Credential is still accepted; the warning nudges users toward api_key_cmd / auth_token_cmd so secrets are not at rest in JSON. - Extend .gitattributes with explicit binary markers for web fonts (woff, woff2, ttf, otf, eot), WebAssembly (.wasm), compressed archives (.zip, .tar, .gz, .tgz), and other image formats (.webp, .bmp, .tiff) to prevent CRLF corruption on Windows clones. * fix(keycmd): tighten validateKeyCmd per review feedback Three bugs flagged by the code review bot: - Backtick regex was backtick-space, missing the common case of`cmd` with no space. Changed to match any backtick character. - Command-separator [;|&] and redirection \d*>>?\s*\S were too broad: they rejected existing tests that use semicolons, ampersands, and 2>/dev/null, which are legitimate in helpers that suppress stderr or chain commands. Both patterns are removed; only backtick and null byte checks remain, each with zero plausible false-positives. - Custom min() shadowed Go built-in min (Go 1.21+). go.mod declares go 1.25.5, so removed the custom helper and inlined the bounds check. * security: gate plaintext credential warnings and add tests Address review feedback on the credential-hardening change: - Emit the plaintext api_key / auth_token warning only when no corresponding api_key_cmd / auth_token_cmd is configured. This removes the duplicate warning when both a static key and a command were set, and limits the nudge to configs that actually lack a secret-manager path. The legacy check now also runs after the completeness guard so an incomplete llm block no longer warns spuriously. - Trim the oversized suspiciousPatterns / validateKeyCmd comments to the essentials. - Add tests: validateKeyCmd backtick / NUL rejection (unit and through resolveKeyCmd), and the plaintext-warning fire / suppress paths for both the provider and legacy config. * Revert plaintext-credential warning from #1291 The nudge printed a stderr WARNING on every run for the common, supported case of a static api_key/auth_token in config, producing unsuppressable noise with no functional benefit and risking CI stderr assertions. Restore silent acceptance of static credentials; the both-set warning and the keycmd backtick/NUL validation are unchanged. --------- Co-authored-by: kite <lizhengfeng.lzf@alibaba-inc.com>
31 lines
455 B
Plaintext
31 lines
455 B
Plaintext
* text=auto eol=lf
|
|
|
|
*.png binary
|
|
*.jpg binary
|
|
*.jpeg binary
|
|
*.gif binary
|
|
*.ico binary
|
|
*.tiktoken binary
|
|
|
|
# Web fonts — must never receive CRLF normalisation
|
|
*.woff binary
|
|
*.woff2 binary
|
|
*.ttf binary
|
|
*.otf binary
|
|
*.eot binary
|
|
|
|
# WebAssembly bytecode
|
|
*.wasm binary
|
|
|
|
# Compressed archives
|
|
*.zip binary
|
|
*.jar binary
|
|
*.tar binary
|
|
*.gz binary
|
|
*.tgz binary
|
|
|
|
# Other binary image formats
|
|
*.webp binary
|
|
*.bmp binary
|
|
*.tiff binary
|