Test OIDC publish path with 2.0.1-rc.1#10
Conversation
There was a problem hiding this comment.
이번 변경은 자바스크립트 패키지 버전을 2.0.1-rc.1로 올려 OIDC 게시 경로를 검증하려는 목적입니다. 코드 변경 자체는 작지만, 현재 게시 워크플로는 릴리스 이벤트에서 명시적으로 기본 태그를 latest로 넘기므로 이 프리릴리스가 npm latest를 덮을 수 있습니다. PR 본문에서 의도한 rc 태그 게시와 실제 워크플로 동작이 어긋나므로 병합 전 수정이 필요합니다.
좋은 점
- 프리릴리스 버전 문자열이 npm 시맨틱 버전 형식에 맞게 설정되어 있습니다.
- publishConfig.provenance가 이미 켜져 있어 OIDC 기반 provenance 게시 경로 검증 의도와 맞습니다.
🔴 반드시 수정할 사항
- 릴리스 이벤트에서 2.0.1-rc.1이 npm latest 태그로 게시되지 않도록 .github/workflows/npm-publish.yml의 태그 결정 로직을 수정해야 합니다. 예를 들어 package.json 버전에 하이픈이 포함되면 rc 또는 next 태그를 사용하고, 안정 버전일 때만 latest를 사용하도록 분기하는 방식이 안전합니다.
💡 권장 개선 사항
- 프리릴리스 게시 정책을 워크플로에 테스트 가능한 형태로 고정하면 이후 rc/beta 버전에서 같은 실수를 줄일 수 있습니다. 최소한 태그 계산 셸 로직을 별도 단계로 두고 package.json 버전이 prerelease일 때 latest를 거부하도록 검증하는 편이 좋습니다.
⚠️ 인라인 게시에서 제외된 지적 1건 (RIGHT-side diff 밖 · GitHub 422 방어) — 펼쳐 보기
.github/workflows/npm-publish.yml:86— [Major] 문제 → 릴리스 이벤트에서는 inputs.tag가 없어서 TAG가 비어 있고, 이 명령은 기본값 latest를 명시적으로 넘깁니다. 영향 → package.json의 2.0.1-rc.1을 GitHub Release로 게시하면 PR 본문 의도와 달리 npm latest가 프리릴리스로 이동할 수 있습니다. 제안 → npm publish 전에 package.json 버전을 읽어 하이픈이 포함된 프리릴리스면 rc 또는 next 태그를 사용하고, 안정 버전일 때만 latest를 사용하도록 태그를 계산하세요.
리뷰 모델: gpt-5.5
There was a problem hiding this comment.
OIDC 배포 파이프라인을 검증하기 위해 패키지 버전을 rc로 올린 것을 확인했습니다. 다만 현재의 GitHub Actions 설정상 GitHub Release 이벤트를 통해 배포가 진행될 경우, 의도와 달리 npm에 latest 태그로 퍼블리시될 위험이 있으므로 주의가 필요합니다.
좋은 점
- OIDC 기반의 npm 배포 파이프라인 연동을 정식 릴리스 전에 미리 rc 버전을 활용하여 엔드투엔드 환경에서 안전하게 검증하려는 접근 방식이 좋습니다.
개선할 점
.github/workflows/npm-publish.yml개선 제안: 현재 배포 스텝은 GitHub Release 발생 시inputs.tag가 비어있어 무조건latest태그로 배포하도록 동작합니다. GitHub API를 통해prerelease여부를 확인하거나package.json의 버전 문자열에 하이픈(-)이 포함되었는지 판별하여next나rc태그를 동적으로 부여하도록 CI 스크립트를 개선하면 향후 배포 실수를 예방할 수 있습니다.
기술 단위 코멘트 1건은 각 라인에 별도 표시됩니다.
리뷰 생성 모델: gemini-3.1-pro-preview
| { | ||
| "name": "@swift-man/material-design-color", | ||
| "version": "2.0.0", | ||
| "version": "2.0.1-rc.1", |
There was a problem hiding this comment.
[Major] PR 본문에서 'rc' 태그 배포를 의도하셨으나, 기존처럼 GitHub Release 이벤트를 통해 이 버전을 배포하게 되면 워크플로우의 ${TAG:-latest} 폴백 처리로 인해 npm에 latest 태그가 덮어씌워질 위험이 큽니다. 이번 배포는 기존 latest 보호를 위해 가급적 GitHub Actions 탭에서 workflow_dispatch 이벤트를 이용해 tag 파라미터에 rc를 직접 입력하여 수동 실행하시길 권장합니다.
Codex and Gemini both flagged that the publish workflow defaulted to publishing under `latest` for any release event — so cutting v2.0.1-rc.1 to validate the OIDC path would have displaced the stable v2.0.0 for every consumer doing `npm install <pkg>`. Add a Resolve npm dist-tag step that inspects package.json version: anything containing a `-` (semver prerelease syntax) publishes under `next`, plain stable versions stay on `latest`. workflow_dispatch keeps an explicit override path for cases like `beta` or `canary`, but its default is now empty so the auto rule applies unless the maintainer overrides on purpose. Logic verified locally: 2.0.1-rc.1 → next 2.0.1 → latest 2.0.0-beta.5+build.1 → next Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
버전 번호와 dist-tag 설정에 대한 변경이 있습니다. 버전 번호는 RC 버전으로 변경되었으며, dist-tag 설정이 자동으로 결정되도록 수정되었습니다.
반드시 수정할 사항
- 버전 번호가 RC 버전으로 변경되었으나, 실제 배포 시에는 stable 버전이어야 합니다.
- dist-tag 설정이 자동으로 결정되도록 수정되었습니다. RC 버전이 배포될 경우 stable 버전이 아닌 next 버전으로 배포될 수 있습니다.
권장 개선사항
- dist-tag 설정에 대한 주석을 더 자세히 작성하여 사용자가 쉽게 이해할 수 있도록 합니다.
개선된 점
- dist-tag 설정이 자동으로 결정되도록 수정되었습니다.
라인 단위 코멘트
- 자동 리뷰에서 1개의 라인 단위 개선 사항을 남겼습니다.
사용된 모델: mlx-community/Qwen2.5-Coder-7B-Instruct-4bit
| # already-published version, use `npm dist-tag add` outside this workflow. | ||
| description: "Dist-tag for the new version being published (e.g. next, beta). Does not retag existing versions." | ||
| # Leave blank to auto-detect: semver prereleases (anything with a `-` | ||
| # in the version) publish under `next`, stable versions under `latest`. |
There was a problem hiding this comment.
[Major] dist-tag 설정이 자동으로 결정되도록 수정되었습니다. RC 버전이 배포될 경우 stable 버전이 아닌 next 버전으로 배포될 수 있습니다.
There was a problem hiding this comment.
OIDC 게시 경로를 보호하려는 방향은 좋지만, 현재 워크플로가 요구하는 npm 버전을 실제로 만족하지 못할 가능성이 큽니다. 이 상태로는 토큰 없는 npm publish 검증이 실패할 수 있어 병합 전 수정이 필요합니다. prerelease를 next 태그로 자동 분기하고 release 태그와 package.json 버전을 검증하는 흐름은 적절합니다.
좋은 점
- release 이벤트에서 태그 버전과 package.json 버전을 비교해 잘못된 버전 게시를 막은 점
- prerelease 버전을 latest가 아닌 next 태그로 자동 분기한 점
- 게시 전에 타입 체크, 빌드, 패키지 exports 경로 스모크 테스트를 실행하는 점
🔴 반드시 수정할 사항
- npm Trusted Publishers는 npm 11.5.1 이상이 필요한데, node-version: "22"만으로는 해당 npm 버전이 보장되지 않습니다. Node 22 계열은 npm 10.x가 번들된 릴리스가 있어 OIDC publish가 인증 단계에서 실패할 수 있으므로 Node 24로 올리거나 npm@11.5.1 이상을 명시적으로 설치하고 버전 검증을 추가해야 합니다.
⚠️ 인라인 게시에서 제외된 지적 1건 (RIGHT-side diff 밖 · GitHub 422 방어) — 펼쳐 보기
.github/workflows/npm-publish.yml:40— [Major] 문제: npm Trusted Publishers는 npm 11.5.1 이상이 필요한데,node-version: "22"는 해당 npm 버전을 보장하지 않습니다. 영향: 현재 Node 22 계열 릴리스는 npm 10.x가 번들될 수 있어npm publish가 OIDC 토큰 교환을 수행하지 못하고 게시 검증이 실패할 수 있습니다. 제안:node-version을 npm 11.5.1 이상이 포함된 Node 24로 올리거나, setup 이후npm install -g npm@>=11.5.1및npm --version검증 단계를 추가하세요.
리뷰 모델: gpt-5.5
There was a problem hiding this comment.
OIDC(Trusted Publishers)를 도입하여 보안성이 높은 npm 배포 파이프라인을 구성한 점이 매우 훌륭합니다. 다만, actions/setup-node의 레지스트리 설정이 OIDC 토큰 방식과 충돌하여 npm 파싱 오류를 유발하는 치명적인 이슈가 있어 수정이 필요합니다.
좋은 점
- GitHub Actions에서
git merge-base --is-ancestor를 통해 릴리스 커밋의 무결성(main 브랜치 포함 여부)을 배포 전에 검증하여 안전성을 확보한 점이 매우 견고합니다. - Node의 패키지 self-referencing 메커니즘을 이용한 smoke 테스트를 워크플로우에 포함하여, ESM 호환성 및 경로 맵핑 회귀를 배포 전에 미리 검증하는 접근이 훌륭합니다.
개선할 점
- OIDC를 활용한 인증 모델은 토큰 노출 위험을 없애주어 보안성이 뛰어나지만, 최신 배포 방식을 도입할 때는 기존 인증 방식의 잔재(예:
.npmrc생성을 강제하는 레지스트리 옵션)를 완전히 제거하여 시스템의 불필요한 충돌을 예방해야 합니다.
(주: 다음 1개 코멘트는 PR diff 범위 밖이라 본문에 모았습니다.)
드롭된 라인 지적
.github/workflows/npm-publish.yml:41— [Critical]actions/setup-node에registry-url을 설정하면 내부적으로.npmrc파일에_authToken=${NODE_AUTH_TOKEN}구문이 생성됩니다. OIDC 인증을 사용할 경우 워크플로우에서NODE_AUTH_TOKEN환경변수를 전달하지 않기 때문에, npm(버전 10 이상)은 설정 파싱 단계에서Failed to replace env in config: ${NODE_AUTH_TOKEN}에러를 발생시키고 배포가 즉시 실패합니다. OIDC 기반 인증 시에는 npm 기본 레지스트리가 자동으로 적용되므로 해당registry-url설정을 제거해야 오류 없이 인증 교환이 이루어집니다.
리뷰 생성 모델: gemini-3.1-pro-preview
Codex flagged that pinning node-version: "22" doesn't actually guarantee npm 11+ — Node 22.0–22.9 shipped npm 10.x, and actions/setup-node could legitimately resolve to one of those patches. Without npm 11.5.1+ the OIDC token exchange never runs and publish would fail at the auth step with no static token to fall back on. Move both workflows to Node 24 LTS, which has shipped npm 11.x across its entire patch range. Add an explicit "Verify npm version" guard step in the publish workflow so a future Node 24 patch with an older bundled npm fails loudly with a useful error instead of silently breaking publish. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
릴리스 전에 JS 패키지를 타입 검사, 빌드, 스모크 테스트까지 거친 뒤 OIDC로 발행하도록 경로를 잘 보강했습니다. 프리릴리스 버전을 자동으로 next 태그로 보내는 처리도 latest 오염을 막는 데 직접적으로 도움이 됩니다. 다만 현재 변경 범위에서 병합을 막을 만한 버그나 보안 문제는 확인하지 못했습니다.
좋은 점
- 프리릴리스 여부를 패키지 버전에서 판별해 기본 배포 태그를 분기한 점
- 릴리스 태그와 package.json 버전 일치를 발행 전에 검증한 점
- 발행 워크플로와 CI 모두에서 JS 패키지의 타입 검사, 빌드, 스모크 테스트를 실행하도록 맞춘 점
리뷰 모델: gpt-5.5
There was a problem hiding this comment.
npm publish workflow를 업데이트했습니다. Node.js 버전을 24로 변경하고, dist-tag를 자동으로 결정하는 스크립트를 추가했습니다.
반드시 수정할 사항
- dist-tag override에 대한 설명이 부족합니다. 사용자가 dist-tag를 명시적으로 지정할 수 있도록 설명해야 합니다.
권장 개선사항
- npm publish 스크립트에서 OIDC trusted publishers의 요구사항을 명시적으로 언급할 수 있습니다.
개선된 점
- Node.js 버전을 24로 업데이트하여 최신 기능을 사용할 수 있습니다.
- dist-tag를 자동으로 결정하는 스크립트를 추가하여 버전 관리가 편리해졌습니다.
라인 단위 코멘트
- 자동 리뷰에서 1개의 라인 단위 개선 사항을 남겼습니다.
사용된 모델: mlx-community/Qwen2.5-Coder-7B-Instruct-4bit
| # already-published version, use `npm dist-tag add` outside this workflow. | ||
| description: "Dist-tag for the new version being published (e.g. next, beta). Does not retag existing versions." | ||
| # Leave blank to auto-detect: semver prereleases (anything with a `-` | ||
| # in the version) publish under `next`, stable versions under `latest`. |
There was a problem hiding this comment.
[Major] dist-tag override에 대한 설명이 부족합니다. 사용자가 dist-tag를 명시적으로 지정할 수 있도록 설명해야 합니다.
There was a problem hiding this comment.
OIDC를 활용한 npm 배포 파이프라인이 안전하고 견고하게 구현되었습니다. NPM_TOKEN 의존성을 제거한 점과 셸 인젝션을 방어한 스크립트 작성 방식이 아주 훌륭합니다. 몇 가지 사소한 스크립트 개선 제안을 코멘트로 남겼습니다.
좋은 점
id-token: write권한과 OIDC 토큰 교환 메커니즘을 적용하여 장기 인증 정보(NPM_TOKEN) 없이 보안 수준이 높은 npm publish 자동화를 구축한 점이 훌륭합니다.${{ inputs.tag }}와 같은 사용자 입력을 셸 스크립트에 직접 보간(Interpolation)하지 않고env를 경유하여 셸 인젝션 취약점을 원천 차단한 설계가 아주 안전합니다.*-*패턴 매칭을 통해next와latest배포 태그를 자동 라우팅하고, workflow_dispatch로 수동 오버라이드를 허용하는 유연한 파이프라인 설계가 좋습니다.
개선할 점
- 현재 스크립트 기반 검증(npm 버전, package.json 태그 일치 여부 등) 로직이 잘 작성되어 있으나, 향후 파이프라인이 더 복잡해질 경우 이 로직들을 별도의 실행 가능한 스크립트 파일로 분리하면 유지보수와 테스트가 더 쉬워질 것입니다.
기술 단위 코멘트 1건은 각 라인에 별도 표시됩니다.
(주: 다음 1개 코멘트는 PR diff 범위 밖이라 본문에 모았습니다.)
드롭된 라인 지적
.github/workflows/npm-publish.yml:63— [Minor]actions/checkout@v4스텝에서fetch-depth: 0옵션을 사용하여 이미 전체 히스토리를 가져온 상태이므로, 여기서 원격origin을 다시fetch할 필요 없이 바로 로컬의origin/main참조를 사용할 수 있습니다. 불필요한 네트워크 I/O를 줄일 수 있습니다.
리뷰 생성 모델: gemini-3.1-pro-preview
| PKG_VERSION=$(node -p "require('./package.json').version") | ||
| if [ -n "$INPUT_TAG" ]; then | ||
| DIST_TAG="$INPUT_TAG" | ||
| elif [[ "$PKG_VERSION" == *-* ]]; then |
There was a problem hiding this comment.
[Suggestion] Bash의 [[ ... == *-* ]] 구문도 훌륭하지만, 버전 문자열 검증을 더 엄격하게 수행하려면 [[ "$PKG_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+-.*$ ]] 형태의 정규식을 활용하여 예기치 않은 비표준 버전 포맷에 대한 라우팅 오류를 방어하는 것도 좋은 방법입니다.
Cut a prerelease to verify the OIDC trusted publisher works end-to-end. The 'rc' tag keeps 'latest' on 2.0.0.