fix: add supply-chain integrity and debug log hardening#9
Merged
Palbahngmiyine merged 5 commits intomainfrom Apr 16, 2026
Merged
fix: add supply-chain integrity and debug log hardening#9Palbahngmiyine merged 5 commits intomainfrom
Palbahngmiyine merged 5 commits intomainfrom
Conversation
Addresses supply-chain and data-exposure risks identified by security audit: - upgrade: verify SHA256 checksum of downloaded archive against checksums.txt - upgrade: validate download URL points to trusted GitHub hosts (HTTPS only) - install.sh: verify SHA256 checksum before extracting (sha256sum/shasum) - client: redact senderKeys, groupKeys, secretKey, apiSecret in debug logs - gitignore: add .claude/ to prevent accidental commit of local settings Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves the security and reliability of the upgrade process and logging. Key changes include the implementation of trusted host validation and SHA256 checksum verification for binary downloads in both the upgrade command and the installation script. Furthermore, a new redaction utility was added to pkg/client to mask sensitive API keys and secrets in debug logs. Feedback was provided to refine the redaction regular expression to better handle escaped characters within JSON strings.
| // sensitiveFieldPattern matches JSON fields that should be redacted in debug logs. | ||
| // Covers senderKeys, groupKeys, and similar secret fields from API responses. | ||
| var sensitiveFieldPattern = regexp.MustCompile( | ||
| `"(senderKey[s]?|groupKey[s]?|secretKey|apiSecret)":\s*"[^"]*"`, |
There was a problem hiding this comment.
현재 정규표현식 "[^"]*"는 JSON 값 내부에 이스케이프된 따옴표(\")가 포함된 경우 이를 값의 끝으로 오해하여 마스킹이 불완전하게 수행될 수 있습니다. 민감한 데이터에 특수문자가 포함될 가능성에 대비하여, 이스케이프 시퀀스를 올바르게 처리할 수 있는 정규표현식(예: "(?:[^"\\\\]|\\\\.)*")을 사용하는 것이 더 안전합니다.
Suggested change
| `"(senderKey[s]?|groupKey[s]?|secretKey|apiSecret)":\s*"[^"]*"`, | |
| `"(senderKey[s]?|groupKey[s]?|secretKey|apiSecret)":\s*"(?:[^"\\]|\\.)*"`, |
- Validate checksums.txt URL against trusted-host allowlist (CRITICAL) - Add redirect-checking HTTP client to prevent allowlist bypass (CRITICAL) - Replace mutable allowedDownloadHosts slice with immutable isAllowedDownloadHost func (HIGH) - Validate SHA256 hash format (64 hex chars) before comparison (HIGH) - Apply redaction before truncation to prevent partial secret exposure (HIGH) - Add empty hash guard in install.sh (HIGH) - Add apiKey to sensitive field redaction pattern (MEDIUM) - Add subdomain bypass test cases for URL validation (MEDIUM) - Add hash format validation and case-insensitive hash tests (MEDIUM) - Remove dead code in setupUpgradeServer (MEDIUM) - Improve comment accuracy for sensitiveFieldPattern and verifyChecksum (MEDIUM) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Reject non-standard ports in URL validation and redirect checking - Replace regex-based redaction with JSON-aware recursive redaction (handles array/object values, not just strings) - Replace grep regex with awk exact-field match in install.sh - Add port bypass and array/object redaction tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GitHub redirects release asset downloads through this domain. Without it, the redirect-checking HTTP client would reject legitimate downloads. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
redactSensitiveFields now supports both top-level objects and
top-level arrays (e.g. [{"apiKey":"secret"}]). Previously only
map responses were redacted; array responses fell through unredacted.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
checksums.txt기반 SHA256 체크섬 검증 추가github.com,objects.githubusercontent.com)인지 HTTPS 스킴과 함께 검증sha256sum/shasum자동 감지)--debug모드에서 API 응답 로깅 시senderKeys,groupKeys,secretKey,apiSecret필드를[REDACTED]로 마스킹.claude/디렉토리 추가 (로컬 설정 실수 커밋 방지)Test plan
go test ./...전체 통과 (11 패키지)go vet ./...통과go build ./...성공bash -n scripts/install.sh문법 검증 통과TestValidateAssetURL— URL 검증 (6 케이스: 신뢰 호스트, http 거부, 악성 호스트 거부 등)TestVerifyChecksum_Valid/_Mismatch/_MissingChecksumsTxt/_AssetNotInChecksums— 체크섬 검증 (4 케이스)TestUpgrade_UntrustedURLRejected— 신뢰할 수 없는 URL로 업그레이드 시도 거부TestUpgrade_ChecksumMismatchRejected— 체크섬 불일치 시 업그레이드 거부TestRedactSensitiveFields— 민감 필드 마스킹 (9 케이스)🤖 Generated with Claude Code