feat(auth): GitHub App installation token을 지원하는 auth login 추가#284
Open
amondnet wants to merge 3 commits into
Open
feat(auth): GitHub App installation token을 지원하는 auth login 추가#284amondnet wants to merge 3 commits into
amondnet wants to merge 3 commits into
Conversation
`gh please auth`를 추가해 `gh auth`를 미러링하면서 GitHub App installation token 발급/재발급을 지원한다. - `auth login`: App 플래그가 없으면 `gh auth login`을 그대로 미러링(stdio 상속, 대화형 플로우 유지). `--app-id`/`--private-key`와 함께 사용하면 RS256 JWT를 서명(node:crypto, 무의존성)해 installation token을 발급한다. - 토큰 전달: 기본은 `gh auth login --with-token`으로 저장, `--print-token`은 CI용으로 표준 출력에 인쇄. - installation 해석: `--installation-id` → `--owner`(org→user 조회) → 단일 설치 자동 선택. - private key 소스: `--private-key <path>`, `-`(stdin), `GH_APP_PRIVATE_KEY`. - 자동 갱신(재발급): App 설정을 `~/.please/auth.json`(0600, 비밀 값 미저장)에 저장하고 `auth token`(온디맨드 재발급)과 `auth git-credential`(git 작업 시 투명 갱신) helper 제공. - GHES 지원(`--hostname` → /api/v3). 테스트: github-app(JWT 서명 검증, installation 해석, 토큰 발급, 키 해석), auth-config(0600 권한, 비밀 값 미저장 라운드트립), auth 명령 등록. docs: ADR 0008 및 AUTH_WORKFLOW.md 추가.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds GitHub App authentication commands, token minting and persistence helpers, git credential helper support, localized auth messages, and docs for the workflow. ChangesGitHub App authentication
Sequence Diagram(s)sequenceDiagram
participant createAuthLoginCommand
participant runAppLogin
participant resolvePrivateKey
participant generateAppJwt
participant resolveInstallationId
participant createInstallationToken
participant gh
participant git
createAuthLoginCommand->>runAppLogin: dispatch App mode
runAppLogin->>resolvePrivateKey: load private key
runAppLogin->>generateAppJwt: sign App JWT
runAppLogin->>resolveInstallationId: resolve installation
runAppLogin->>createInstallationToken: mint installation token
runAppLogin->>gh: auth login --with-token
runAppLogin->>git: credential helper setup or hint
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new gh please auth command group, enabling support for GitHub App installation tokens. The changes include logic for JWT signing, token minting, configuration persistence, and a git credential helper. The review comments identify several areas for improvement, including ensuring configuration persistence when using environment variables, preventing potential deadlocks in sub-process execution, and strengthening security through proper file permission handling and path traversal prevention.
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 2 high |
| Security | 6 high |
🟢 Metrics 172 complexity · 4 duplication
Metric Results Complexity 172 Duplication 4
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Greptile SummaryThis PR adds
Confidence Score: 4/5Safe to merge after fixing the git credential exit-code behavior; all other paths are well-structured and tested. The git credential helper exits with code 1 when no App config is saved, which per the git credential protocol prevents fallthrough to other helpers or interactive prompting — making all git operations on the configured host fail after config loss. The rest of the implementation (JWT signing, installation resolution, config hardening, preflight guards, absolute path persistence) is correct and covered by 36 new tests. src/commands/auth/git-credential.ts — the no-config code path in Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant U as User/CI
participant L as auth login
participant GA as github-app.ts
participant GH as GitHub API
participant AC as auth-config.ts
participant GC as gh CLI
Note over U,GC: gh please auth login --app-id … --private-key …
U->>L: invoke with App flags
L->>GA: resolvePrivateKey(path/env/stdin)
L->>GA: generateAppJwt(appId, privateKey)
L->>GA: resolveInstallationId(jwt, owner?)
GA->>GH: "GET /orgs/{owner}/installation"
GH-->>GA: installation id
L->>GA: createInstallationToken(jwt, installationId)
GA->>GH: "POST /app/installations/{id}/access_tokens"
GH-->>GA: token + expires_at
L->>GC: gh auth login --with-token (stdin pipe)
L->>AC: writeAuthConfig(appId, installationId, privateKeyPath) 0600
Note over U,GC: gh please auth token (re-mint)
U->>AC: readAuthConfig()
AC-->>U: config (or null → mirror gh auth token)
U->>GA: resolvePrivateKey + generateAppJwt + createInstallationToken
GA->>GH: "POST /app/installations/{id}/access_tokens"
GH-->>GA: fresh token
GA-->>U: stdout: token
Note over U,GC: git push (git-credential get)
U->>GA: handleGet() — read stdin protocol/host
GA->>AC: readAuthConfig()
AC-->>GA: config (or null/mismatch → exit 0, no output)
GA->>GH: "POST /app/installations/{id}/access_tokens"
GH-->>GA: fresh token
GA-->>U: "stdout: username=x-access-token\npassword=token"
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant U as User/CI
participant L as auth login
participant GA as github-app.ts
participant GH as GitHub API
participant AC as auth-config.ts
participant GC as gh CLI
Note over U,GC: gh please auth login --app-id … --private-key …
U->>L: invoke with App flags
L->>GA: resolvePrivateKey(path/env/stdin)
L->>GA: generateAppJwt(appId, privateKey)
L->>GA: resolveInstallationId(jwt, owner?)
GA->>GH: "GET /orgs/{owner}/installation"
GH-->>GA: installation id
L->>GA: createInstallationToken(jwt, installationId)
GA->>GH: "POST /app/installations/{id}/access_tokens"
GH-->>GA: token + expires_at
L->>GC: gh auth login --with-token (stdin pipe)
L->>AC: writeAuthConfig(appId, installationId, privateKeyPath) 0600
Note over U,GC: gh please auth token (re-mint)
U->>AC: readAuthConfig()
AC-->>U: config (or null → mirror gh auth token)
U->>GA: resolvePrivateKey + generateAppJwt + createInstallationToken
GA->>GH: "POST /app/installations/{id}/access_tokens"
GH-->>GA: fresh token
GA-->>U: stdout: token
Note over U,GC: git push (git-credential get)
U->>GA: handleGet() — read stdin protocol/host
GA->>AC: readAuthConfig()
AC-->>GA: config (or null/mismatch → exit 0, no output)
GA->>GH: "POST /app/installations/{id}/access_tokens"
GH-->>GA: fresh token
GA-->>U: "stdout: username=x-access-token\npassword=token"
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
src/commands/auth/git-credential.ts:37-41
**`get` exits with code 1 when no config, breaking git's fallthrough**
When `readAuthConfig()` returns null, `handleGet` throws, which is caught by the outer `try/catch` and causes `process.exit(1)`. Per the git credential protocol, a helper that has no credentials for the request should exit 0 with no output — that signals "I don't know, ask the next helper." Exiting with a non-zero code signals an error, which causes git to abort the operation rather than fall through to another helper or interactive prompting.
Concretely: if a user's `~/.please/auth.json` is deleted or corrupted (e.g., after restoring dotfiles on a new machine), every `git push`/`git fetch` to the configured host will fail with the `gh-please` error message instead of falling back to the system keychain or prompting for credentials. The protocol/host mismatch paths already handle this correctly by returning silently — the no-config case should do the same.
Reviews (3): Last reviewed commit: "chore(auth): reject blank required field..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (2)
test/lib/auth-config.test.ts (1)
54-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert against the serialized file, not
readAuthConfig()again.This test never reads
Bun.file(path), so it only rechecks the parsed object shape. Extra persisted fields can still slip through unnoticed unless you inspect the JSON written to disk.Suggested fix
- test('should never persist a private key value', () => { + test('should never persist a private key value', async () => { const path = makeTmpPath() writeAuthConfig({ appId: '1', installationId: '2', privateKeyPath: '/keys/app.pem' }, path) - const raw = Bun.file(path) - // privateKeyPath(경로 참조)만 저장되고 PEM 본문 키가 없어야 한다 - const stored = readAuthConfig(path)! - expect(stored.privateKeyPath).toBe('/keys/app.pem') - expect(Object.keys(stored)).not.toContain('privateKey') - void raw + const stored = await Bun.file(path).json() as Record<string, string> + expect(stored).toEqual({ + appId: '1', + installationId: '2', + privateKeyPath: '/keys/app.pem', + }) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/lib/auth-config.test.ts` around lines 54 - 63, The test in auth-config.test.ts is only validating the parsed object from readAuthConfig() and not the serialized file written by writeAuthConfig(). Update the should never persist a private key value test to read and assert against the contents of the file at the path created by makeTmpPath(), using the existing writeAuthConfig and readAuthConfig symbols only as needed for setup, so persisted privateKey data would be caught even if the parser hides it.test/commands/auth/index.test.ts (1)
23-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd behavior tests for the new auth subcommands.
These assertions only cover names/descriptions. The risky branches added here—saved-config minting vs passthrough in
src/commands/auth/token.ts, plus the credential-helper stdout/error paths insrc/commands/auth/git-credential.ts—still aren't exercised, so regressions in the actual auth flow will slip through.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/commands/auth/index.test.ts` around lines 23 - 41, Add behavior tests for the new auth subcommands instead of only checking command metadata. Cover the real branches in createAuthTokenCommand and createGitCredentialCommand by asserting saved-config minting versus passthrough in src/commands/auth/token.ts, and the stdout/error credential-helper flows in src/commands/auth/git-credential.ts. Use the existing command factory names to locate the logic and verify the expected runtime outcomes, not just name() and description().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 189-195: The authentication help text is incorrectly implying that
setup-git is a passthrough auth subcommand; update the built-in auth section so
the option is described only on gh please auth login, matching
src/commands/auth/login.ts. Keep the login command line as the place where
--setup-git is mentioned, and remove it from the generic gh please auth
<status|logout|refresh|...> passthrough wording.
In `@docs-dev/AUTH_WORKFLOW.md`:
- Around line 113-128: The fenced diagram block in the auth workflow doc is
missing a language label, causing the markdownlint MD040 warning. Update the
fenced block around the auth flow diagram to include an appropriate tag such as
text, or convert the diagram to Mermaid if that better fits the content, so the
block is explicitly labeled and markdownlint passes.
- Around line 11-13: The AUTH_WORKFLOW documentation currently lists `setup-git`
as a passthrough auth subcommand, but `gh please auth setup-git` does not exist.
Update the description around `gh please auth login` and its passthrough list to
remove `setup-git`, and align the wording with the behavior implemented in
`src/commands/auth/login.ts` where `--setup-git` belongs to the login command
itself.
In `@src/commands/auth/git-credential.ts`:
- Around line 13-19: handleGet() currently ignores the git credential payload
and always returns a token, so it should first parse the stdin fields and
validate the requested protocol/host before printing anything. Update the
git-credential flow in handleGet() to consume the incoming request, compare it
against the saved auth config used by mintTokenFromSavedConfig(), and only emit
username/password when the request matches; otherwise return no credentials. Use
the existing APP_TOKEN_USERNAME and handleGet() entrypoint to keep the fix
localized.
In `@src/commands/auth/login.ts`:
- Around line 80-100: Persist the key-source metadata in auth/login so
env-backed App logins can be reissued later. In login(), keep track of whether
resolvePrivateKey() came from a file path, stdin, or GH_APP_PRIVATE_KEY instead
of only the raw PEM, and when writing AuthConfig via writeAuthConfig(), store a
source marker for env-backed login rather than omitting the config. Update the
AuthConfig shape and the options.privateKey handling so the saved auth state
contains enough information for gh please auth token to mint again without
persisting secret key material.
- Around line 49-50: The auth failure messages in login flow still hardcode
English prefixes and should be localized through AuthMessages. Update the
failure paths in login.ts around the gh auth login checks so the thrown errors
use i18n-driven message templates, keeping only the dynamic stderr text from the
tool output. Also adjust the user-facing error path that prints these messages
so it consumes the localized AuthMessages output rather than raw English
strings, preserving the existing error details while internationalizing the
prefixes.
In `@src/lib/auth-config.ts`:
- Around line 27-31: The auth config parsing in auth-config.ts only verifies
that appId and installationId exist, but it still blindly casts malformed JSON
to AuthConfig. Update the parsing logic in the config loader so it validates the
full shape of the object (especially privateKeyPath and hostname types, along
with the required fields) before returning AuthConfig; if any field has the
wrong type or is missing, return null instead of allowing the bad object to
reach resolvePrivateKey() or apiBaseUrl().
- Around line 43-47: The auth config write path in the write helper does not
re-enforce restrictive permissions on existing directories/files, so the
promised 0700/0600 settings only apply on first creation. Update the logic
around the dirname/path handling to explicitly set the directory and auth file
modes every time the config is written, using the existing write helper and any
parent-directory creation code so that previously created broader permissions
are corrected on each run.
In `@src/lib/github-app.ts`:
- Line 107: The minting library in github-app.ts is leaking user-facing English
auth text through thrown Error messages, which the command layer prints
verbatim. Replace these throws with structured error codes or i18n keys in the
affected github-app helpers (including the installation token path and the other
auth error sites noted in the diff), then update src/commands/auth/login.ts and
src/commands/auth/token.ts to translate those codes via getAuthMessages() before
displaying output. Keep the library focused on machine-readable errors and let
the command layer own all localized messaging.
- Line 104: The GitHub API calls in createInstallationToken,
lookupOwnerInstallation, and resolveInstallationId are missing an abort/timeout
path, so stalled fetches can hang auth flows indefinitely. Add a shared
timeout/AbortController-based signal to the fetchImpl calls in github-app.ts and
thread it through these request helpers so each request fails fast instead of
waiting forever.
In `@src/lib/i18n.ts`:
- Around line 395-425: Add localized auth-failure messages to
AuthMessages/authMessages so known errors from the auth flow are not printed as
raw English error.message values. Extend getAuthMessages with typed keys for
predictable failures (for example missing saved App config and missing
private-key source), then update src/commands/auth/token.ts and
src/commands/auth/git-credential.ts to map those internal exceptions to the new
i18n strings before logging or printing. Keep the generic
errorPrefix/unknownError as fallback only.
---
Nitpick comments:
In `@test/commands/auth/index.test.ts`:
- Around line 23-41: Add behavior tests for the new auth subcommands instead of
only checking command metadata. Cover the real branches in
createAuthTokenCommand and createGitCredentialCommand by asserting saved-config
minting versus passthrough in src/commands/auth/token.ts, and the stdout/error
credential-helper flows in src/commands/auth/git-credential.ts. Use the existing
command factory names to locate the logic and verify the expected runtime
outcomes, not just name() and description().
In `@test/lib/auth-config.test.ts`:
- Around line 54-63: The test in auth-config.test.ts is only validating the
parsed object from readAuthConfig() and not the serialized file written by
writeAuthConfig(). Update the should never persist a private key value test to
read and assert against the contents of the file at the path created by
makeTmpPath(), using the existing writeAuthConfig and readAuthConfig symbols
only as needed for setup, so persisted privateKey data would be caught even if
the parser hides it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 02d197e5-ed32-4079-a761-7072752cf2b4
📒 Files selected for processing (17)
CLAUDE.mddocs-dev/AUTH_WORKFLOW.mddocs-dev/adr/0008-auth-github-app-installation-tokens.mdsrc/commands/auth/git-credential.tssrc/commands/auth/index.tssrc/commands/auth/login.tssrc/commands/auth/token.tssrc/index.tssrc/lib/app-auth.tssrc/lib/auth-config.tssrc/lib/github-app.tssrc/lib/i18n.tssrc/types.tstest/commands/auth/index.test.tstest/commands/auth/login.test.tstest/lib/auth-config.test.tstest/lib/github-app.test.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 17 files
Confidence score: 2/5
- In
src/commands/auth/login.ts, persisting--private-keyas a relative path and skipping re-mint config when login comes from environment variables can break latergh please auth tokenrefreshes withENOENTor missing config state; this is the most user-visible regression in the auth flow — normalize persisted key paths (or store an absolute/canonical form) and always persist refresh-required config before merging. - In
src/lib/github-app.ts, interpolatingownerinto the URL withoutencodeURIComponent()can send requests to unintended endpoints when special characters or traversal-like input is present, creating both correctness and security risk — URL-encodeownerbefore constructing the API path. - In
src/lib/auth-config.ts, permission hardening does not correct insecure modes on existing config files, so previously broad file access may remain in place despite the protection intent — enforce/chmod owner-only permissions on read/write for existing files as part of migration/loading. - In
src/lib/auth-config.ts(readAuthConfig), accepting truthy non-string required fields allows corrupted config to pass initial checks and fail later at runtime, which can cause hard-to-diagnose auth errors after deployment — add strict type validation for required fields and fail fast with a clear error before merging.
Architecture diagram
sequenceDiagram
participant User as User / CI
participant CLI as gh please auth
participant Lib as GitHub App Lib
participant Config as Auth Config (~/.please/auth.json)
participant Gh as gh CLI (native)
participant API as GitHub API
participant Git as git config
Note over User,Git: Mirror path (no App flags)
User->>CLI: auth login (no --app-id)
CLI->>CLI: detect no App flags, forward raw argv
CLI->>Gh: Bun.spawn(gh auth login ...), inherit stdio
Gh-->>User: interactive TTY prompt
alt User exits
Gh->>CLI: exit code
CLI->>User: process.exit(code)
end
Note over User,Git: NEW: GitHub App installation token login
User->>CLI: auth login --app-id 123 --private-key ./key.pem [--installation-id ... | --owner ...] [--hostname ...] [--print-token] [--setup-git]
CLI->>Lib: resolvePrivateKey({path: './key.pem'})
Lib-->>CLI: privateKey PEM
CLI->>Lib: generateAppJwt({appId, privateKey})
Lib-->>CLI: JWT
alt explicit --installation-id
CLI->>CLI: use given installationId
else --owner provided
CLI->>Lib: resolveInstallationId({jwt, owner, hostname})
Lib->>API: GET /orgs/{owner}/installation
API-->>Lib: 200 { id } or 404
alt 404
Lib->>API: GET /users/{owner}/installation
API-->>Lib: 200 { id }
end
Lib-->>CLI: installationId
else auto-select
CLI->>Lib: resolveInstallationId({jwt})
Lib->>API: GET /app/installations
API-->>Lib: list of installations
alt exactly one
Lib-->>CLI: installationId
else multiple
Lib-->>CLI: throw error with options
end
end
CLI->>Lib: createInstallationToken({jwt, installationId, hostname})
Lib->>API: POST /app/installations/{id}/access_tokens
API-->>Lib: 200 { token, expires_at }
Lib-->>CLI: { token, expiresAt }
alt --print-token
CLI-->>User: token + expiry note to stderr
else default store
CLI->>Gh: Bun.spawn(gh auth login --with-token), pipe token
Gh-->>CLI: exit 0
opt privateKey is file path (not stdin)
CLI->>Config: writeAuthConfig({appId, installationId, privateKeyPath, hostname?})
Config-->>CLI: file written (mode 0600, no secrets)
end
opt --setup-git
CLI->>Git: git config --global credential.https://<host>.helper '!gh-please auth git-credential'
Git-->>CLI: exit 0
end
CLI-->>User: success + hint for git helper
end
Note over User,Git: NEW: Re‑mint token (auth token)
User->>CLI: auth token
CLI->>Config: readAuthConfig()
alt config exists
Config-->>CLI: AuthConfig (appId, installationId, privateKeyPath)
CLI->>Lib: resolvePrivateKey({path: config.privateKeyPath})
Lib-->>CLI: privateKey
CLI->>Lib: generateAppJwt + createInstallationToken
Lib->>API: POST /app/installations/{id}/access_tokens
API-->>Lib: { token }
Lib-->>CLI: token
CLI-->>User: token to stdout
else no config
CLI->>Gh: mirror gh auth token
Gh-->>CLI: token
CLI-->>User: token to stdout
end
Note over User,Git: NEW: Git credential helper (auto‑refresh)
User->>CLI: git (via credential helper) calls: auth git-credential get
CLI->>Lib: mintTokenFromSavedConfig()
Lib->>Config: readAuthConfig()
Config-->>Lib: config
Lib->>Lib: resolvePrivateKey, generateJwt, createToken
Lib->>API: POST /app/installations/{id}/access_tokens
API-->>Lib: { token }
Lib-->>CLI: token
CLI-->>User: stdout: username=x-access-token\npassword=<token>
Note over User,Git: Error handling (applies to all new paths)
alt any step fails (API error, missing key, etc.)
CLI-->>User: error message to stderr, exit 1
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
PR #284 리뷰(gemini/coderabbit/cubic/greptile) 반영: - login: GH_APP_PRIVATE_KEY 환경 변수 로그인 시에도 재발급 설정을 저장하고, private key 경로를 절대 경로로 변환해 다른 CWD에서의 재발급 ENOENT를 방지 - login: --app-id 없이 App 전용 플래그 사용 시 명확한(i18n) 안내 후 종료 - login: 소비하지 않는 stdout 스트림을 'ignore'로 변경(데드락 방지) - github-app: owner를 encodeURIComponent로 인코딩(경로 탐색 방지), installations 목록에 per_page=100 적용, 모든 fetch에 30s 타임아웃 추가 - auth-config: readAuthConfig 필드 타입 검증, 매 쓰기마다 chmod로 0700/0600 강제 - git-credential: 요청 host/protocol이 저장된 설정과 일치할 때만 토큰 제공 (App 토큰이 무관한 호스트로 새는 것 방지) - docs: AUTH_WORKFLOW.md passthrough 목록에서 setup-git 제거(로그인 플래그로 명확화), 다이어그램 코드펜스에 text 라벨 추가 테스트: owner 인코딩, 설정 타입 검증, parseCredentialInput 추가 (43 pass)
There was a problem hiding this comment.
1 issue found across 10 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
cubic 리뷰(2차) 반영: 타입 검증이 빈 문자열을 통과시켜 손상된 auth.json이 유효하게 처리될 수 있던 문제를 수정. appId/installationId가 공백이면 null 반환.
|
Comment on lines
+37
to
+41
| if (!config) { | ||
| throw new Error( | ||
| 'No saved GitHub App credentials found. Run `gh please auth login --app-id <id> --private-key <path>` first.', | ||
| ) | ||
| } |
There was a problem hiding this comment.
get exits with code 1 when no config, breaking git's fallthrough
When readAuthConfig() returns null, handleGet throws, which is caught by the outer try/catch and causes process.exit(1). Per the git credential protocol, a helper that has no credentials for the request should exit 0 with no output — that signals "I don't know, ask the next helper." Exiting with a non-zero code signals an error, which causes git to abort the operation rather than fall through to another helper or interactive prompting.
Concretely: if a user's ~/.please/auth.json is deleted or corrupted (e.g., after restoring dotfiles on a new machine), every git push/git fetch to the configured host will fail with the gh-please error message instead of falling back to the system keychain or prompting for credentials. The protocol/host mismatch paths already handle this correctly by returning silently — the no-config case should do the same.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/commands/auth/git-credential.ts
Line: 37-41
Comment:
**`get` exits with code 1 when no config, breaking git's fallthrough**
When `readAuthConfig()` returns null, `handleGet` throws, which is caught by the outer `try/catch` and causes `process.exit(1)`. Per the git credential protocol, a helper that has no credentials for the request should exit 0 with no output — that signals "I don't know, ask the next helper." Exiting with a non-zero code signals an error, which causes git to abort the operation rather than fall through to another helper or interactive prompting.
Concretely: if a user's `~/.please/auth.json` is deleted or corrupted (e.g., after restoring dotfiles on a new machine), every `git push`/`git fetch` to the configured host will fail with the `gh-please` error message instead of falling back to the system keychain or prompting for credentials. The protocol/host mismatch paths already handle this correctly by returning silently — the no-config case should do the same.
How can I resolve this? If you propose a fix, please make it concise.
|
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.
개요
gh please auth를 추가해gh auth를 미러링하면서 GitHub App installation token 발급·재발급을 지원합니다. 네이티브gh auth login은 사용자(OAuth/PAT) 인증만 지원하고 App installation 인증 경로가 없어, CI·봇·서버 간 자동화에 필요한 짧은 수명(~1시간)의 스코프 토큰을 CLI에서 다룰 수 없었습니다.설계 근거는
docs-dev/adr/0008-auth-github-app-installation-tokens.md, 사용법은docs-dev/AUTH_WORKFLOW.md를 참고하세요.명령 구성
gh please auth login(App 플래그 없음)gh auth login그대로 미러링 (stdio 상속 → 대화형 플로우 유지)gh please auth login --app-id … --private-key …--with-token으로 저장(또는--print-token), 재발급용 설정 저장gh please auth tokengh auth token미러링)gh please auth git-credential <op>gh please auth <status|logout|…>주요 설계 결정
--app-id가 없으면process.argv꼬리를 그대로gh auth login에 넘기고 stdio를 상속해 네이티브 동작·TTY 프롬프트를 100% 보존.node:crypto로 서명(신규 의존성 없음).gh auth login --with-token,--print-token은 CI용(export GH_TOKEN=$(… --print-token)).~/.please/auth.json에 저장하고 온디맨드로 재발급. git credential helper로 git 작업 시 데몬 없이 투명 갱신.auth.json에는 private key 경로 참조만 저장(PEM 본문 미저장), 파일 권한0600. stdin 키는 일회성(재발급 불가).--installation-id→--owner(org→user 조회) → 단일 설치 자동 선택.--hostname→/api/v3.private key 소스 (우선순위)
--private-key -(stdin)--private-key <path>(파일)GH_APP_PRIVATE_KEY환경 변수 (PEM 본문)변경 파일
src/lib/github-app.ts— JWT 서명, installation 해석, 토큰 발급, key 해석src/lib/auth-config.ts—~/.please/auth.json읽기/쓰기(0600, 비밀 값 미저장)src/lib/app-auth.ts— 저장된 설정 기반 재발급 오케스트레이션src/commands/auth/{index,login,token,git-credential}.ts— 명령 구성src/{index,types}.ts,src/lib/i18n.ts— 명령 등록 / 타입 / 메시지(ko·en)테스트
bun run lint:fix,bun run type-check통과test/lib/github-app.test.ts— 실제 RSA 키쌍으로 JWT 서명 검증, installation 해석(org/user/auto/다중/없음), 토큰 발급, key 해석test/lib/auth-config.test.ts— 0600 권한, 비밀 값 미저장 라운드트립, 손상/누락 처리test/commands/auth/*— 명령 등록/플래그/설명남은 작업
Summary by cubic
Adds
gh please auththat mirrorsgh authand adds GitHub App installation token login, re-mint, and a git credential helper with host-scoped auto-refresh. Improves security and reliability (timeouts, strict config perms, safer installation resolution) and supports GHES.New Features
gh please auth login: with--app-idand--private-keymints an installation token;--print-tokenfor CI; otherwise stores viagh auth login --with-token. Optional--setup-gitconfigures the helper.--installation-id→--owner(org→user lookup) → auto-select when only one exists. Private key sources:--private-key <path>,--private-key -(stdin), orGH_APP_PRIVATE_KEY. Config saved to~/.please/auth.json(0600), uses absolute paths, and never persists the PEM; stdin keys are one-shot and not saved.gh please auth tokenre-mints a fresh token from saved config;gh please auth git-credentialmints a new token per git op and only for HTTPS requests to the configured host.--hostname(/api/v3); JWT signed withnode:crypto; all API calls have a 30s timeout.Bug Fixes
--app-id.per_page=100.~/.pleasedir/file permissions are enforced (0700/0600) on every write.Written for commit 839b9e0. Summary will update on new commits.
Summary by CodeRabbit
authcommand group withlogin,token, andgit-credentialhelper support.