Skip to content

docs: 기여 노트 nodejs/node#65659 #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
daeyeon merged 2 commits into from
Aug 30, 2026
Merged

docs: 기여 노트 nodejs/node#65659 #11

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f8ee12
docs: add contribution note for nodejs/node#65659
theSnackOverflow Aug 30, 2026
88f3e96
docs: expand contribution note for nodejs/node#65659
theSnackOverflow Aug 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
176 changes: 176 additions & 0 deletions contributions/65659.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
---
pr-url: https://github.com/nodejs/node/pull/65659
---

## 문제 내용

`--permission-audit`는 [문서상](https://nodejs.org/api/cli.html#--permission-audit) 권한 검사를 수행하되 **차단하지 않고** diagnostics channel로 위반 사항만 발행하는 모드입니다. 그런데 세 경로가 이 계약을 어기고 실제로 차단하고 있었습니다([#65419](https://github.com/nodejs/node/issues/65419), 신고자 @naugtur).

로컬 빌드(v27.0.0-pre)로 재현한 결과입니다.

| API | 플래그 없음 | `--permission` | `--permission-audit` | 판정 |
| --- | --- | --- | --- | --- |
| `fs.statSync` | OK | 차단 | OK | 정상 |
| `fs.lstatSync` | OK | 차단 | **차단** | 버그 |
| `fs.symlinkSync` | OK | 차단 | **차단** | 버그 |
| `process.dlopen` | — | 차단 | **차단** | 버그 |

같은 fs 모듈인데 `stat`은 정상이고 `lstat`은 차단된다는 비대칭이 원인 추적의 실마리였습니다.

### 원인 1 — JS 계층 검사가 audit 모드를 확인하지 않음

C++ 매크로는 audit 모드를 존중합니다(`src/permission/permission.h`).

```cpp
if (!is_granted(...)) { // 여기서 audit 이벤트 발행
if (!warning_only()) { // audit 모드면 throw 생략
ThrowAccessDenied(...);
```

그런데 `lstat`과 `symlink`의 검사는 C++이 아니라 JS 계층에 있고, 두 번째 단계가 없었습니다.

```js
// lib/fs.js
if (permission.isEnabled() && !permission.has('fs.read', path)) {
throw new ERR_ACCESS_DENIED(...); // audit 모드에서도 무조건 throw
}
```

`permission.isEnabled()`는 `--permission || --permission-audit`로 정의되어 audit 모드에서도 `true`입니다. `stat`이 정상이었던 이유는 검사가 C++ 매크로 경로에 있어서였습니다.

### 원인 2 — dlopen은 권한 검사에 도달하기 전에 차단

`src/env.cc`가 audit 모드에도 애드온을 비활성화하고, `DLOpen()`이 audit 인식 매크로보다 **먼저** 그 플래그를 검사합니다.

```cpp
// src/node_binding.cc
if (env->no_native_addons()) {
return THROW_ERR_DLOPEN_DISABLED(...); // audit 모드 무시
}
THROW_IF_INSUFFICIENT_PERMISSIONS(env, kAddon, ""); // 여기까지 도달 못 함
```

에러 코드부터 `ERR_ACCESS_DENIED`가 아닌 `ERR_DLOPEN_DISABLED`이고, **audit 이벤트조차 발행되지 않습니다**. 관측이 목적인 모드에서 관측 자체가 불가능한 상태였습니다.

## 해결 과정과 검증

| 항목 | 내용 |
| --- | --- |
| 분류 태그 | 버그수정 |
| 상태 | Open (2026.08.30 제출, 커밋 `4efc964`) |
| 변경 규모 | 5개 파일, +184 / -7, 커밋 1개 |
| 리뷰 요청 | @nodejs/security-wg (CODEOWNERS 자동 라우팅) |

### 선례 확인 — 헬퍼는 이미 있었다

수정 방향을 정하기 전에 기존 구현을 먼저 뒤졌습니다. `permission.isAuditMode()` 헬퍼가 이미 존재했고, `lib/ffi.js`에서만 쓰이고 있었습니다.

```js
// lib/ffi.js — 올바른 선례
if (permission.has('ffi') || permission.isAuditMode()) {
return;
}
```

`git log -S "isAuditMode" --oneline -- lib/` 결과 커밋이 하나뿐이었습니다(#63047). 즉 헬퍼가 만들어질 때 ffi에만 배선되고 **fs는 누락**된 것이었고, 새 API를 만들 필요 없이 같은 패턴을 확장하면 되는 문제였습니다.

### 수정 1 — JS 계층 6곳

`lib/fs.js` 4곳(`lstat`, `lstatSync`, `symlink`, `symlinkSync`)과 `lib/internal/fs/promises.js` 2곳입니다.

```js
if (permission.isEnabled() && !permission.has('fs.read', path) &&
!permission.isAuditMode()) {
```

**조건 순서가 핵심입니다.** audit 이벤트는 `permission.has()`가 타고 들어가는 C++ `Permission::is_scope_granted()`에서 발행됩니다. `isAuditMode()`를 앞에 두면 단축 평가로 `has()`가 호출되지 않아 이벤트가 사라집니다. 관측 기능을 살리려면 `has()`가 먼저 평가되어야 하고, 이는 `lib/ffi.js`의 기존 패턴과도 일치합니다.

### 수정 2 — src/env.cc

```cpp
if (!options_->allow_addons) {
// In audit mode addon loading must stay enabled: the denial is
// published through the diagnostics channel by the permission
// check in DLOpen() instead of being rejected upfront.
if (!options_->permission_audit) {
options_->allow_native_addons = false;
}
permission()->Apply(this, args, permission::PermissionScope::kAddon);
}
```

`Apply(kAddon)`은 그대로 두었습니다. `kAddon` 스코프가 거부 상태로 남아야 `DLOpen()`의 audit 인식 매크로가 이벤트를 발행하고, `warning_only()`에 의해 throw만 생략되기 때문입니다. 이 한 곳을 고치면 `src/node_worker.cc`의 worker 전파 경로도 함께 해결되어(같은 플래그를 상속) 추가 수정이 불필요했습니다.

### 신규 테스트 2개

기존 `test-permission-audit-fs-does-not-deny.js`의 구조(`spawnSync` + 자식 프로세스에서 `dc.subscribe` + stdout 파싱)를 그대로 따랐습니다.

- `test-permission-audit-fs-lstat-symlink-does-not-deny.js` — audit 모드에서 통과 + 이벤트의 `permission`/`resource` 단언
- `test-permission-audit-addons-does-not-deny.js` — audit 모드에서 `ERR_DLOPEN_DISABLED`가 아닌 `ERR_DLOPEN_FAILED`인지 단언(실제 `.node` 파일 없이 검증 가능)

각 테스트에 **`--permission` 강제 모드 단언을 대조군으로** 넣었습니다. 완화가 강제 모드로 새지 않았다는 증명이 없으면 보안 관련 변경은 리뷰를 통과하기 어렵다고 판단했습니다.

### 검증 순서

문서(`doc/contributing/writing-tests.md`)의 규칙을 따르되, 순서를 의도적으로 잡았습니다.

1. **수정 전 바이너리에서 신규 테스트를 먼저 실행** — audit 케이스 3개만 실패하고 강제 모드 대조군 3개는 통과. 테스트가 실제로 이 버그를 잡는다는 증거를 먼저 확보했습니다.
2. 수정 후 3-모드 재현 매트릭스 재실행 — audit 모드가 플래그 없는 기준선과 완전히 일치, 강제 모드는 무변화
3. `python3 tools/test.py --mode=release -J "parallel/test-permission*"` — 75/75 통과
4. `ffi/test-ffi-permission-audit`, `parallel/test-fs-symlink*` — 통과
5. `make test JOBS=14` 전체 스위트 — cctest 222개, jstest 5,942개, 실패 0
6. `eslint`(JS 4파일), `python3 tools/cpplint.py src/env.cc` — 통과

### 판단이 필요했던 지점 — CVE 관련 코드 완화

`symlink` 제한은 [CVE-2025-55130](https://github.com/nodejs/node/commit/3296b2e734c) 대응으로 들어간 코드였습니다(2025-11, 비공개 보안 PR). 보안 조치를 완화하는 변경이라 근거가 필요했습니다.

`git log`로 시점을 확인한 결과, 해당 CVE 커밋은 audit 모드 도입(#61869, 2026-05)보다 **앞서 있어 audit을 고려할 수 없었던** 상태였습니다. 의도적 예외가 아니라 시점상 누락으로 판단했고, 이 근거와 함께 **반대 대안**(symlink만 예외로 유지하고 문서에 명시)도 PR 본문에 병기해 security-wg가 판정할 수 있게 했습니다.

### 신고자 응대

이슈 본문에 신고자가 "직접 고치고 싶다, 포인터가 필요하다"고 적어둔 상태였고 열흘간 답이 없었습니다. 선점 문제를 피하기 위해 이슈에 **원인 분석 코멘트를 먼저 올리고**(요청받은 포인터), PR 본문과 코멘트 양쪽에 "직접 작업 중이셨다면 기꺼이 양보하겠다"는 의사를 명시했습니다.

## 배운 점

### 1. 한 함수만 읽고 내린 판단은 뒤집힐 수 있다

이번 조사 중, 앞서 머지된 제 문서 PR(#64888)이 틀렸다고 판단한 순간이 있었습니다. `Stream::SendStopSending()`을 보면 `EndReadable()`이 방향 검사보다 먼저 무조건 실행되기 때문입니다.

```cpp
void Stream::SendStopSending(error_code code) {
EndReadable(); // 무조건 실행되는 것처럼 보임
if (!is_pending()) {
if (is_local_unidirectional()) return;
```

한 겹 더 내려가 보니 `is_readable()`이 로컬 단방향 스트림에 `false`를 반환하고, `EndReadable()`의 첫 줄이 `if (!is_readable()) return;`이었습니다. **결국 문서가 맞았습니다.**

Node 코어에서 `is_readable()`, `is_writable()`, `is_pending()` 같은 술어 함수는 조건을 감추는 자리입니다. 호출 그래프를 한 겹만 보고 내린 결론은 정의까지 펼치면 반대로 뒤집힐 수 있다는 걸 실제로 겪었습니다.

### 2. 이슈 목록의 빈자리는 대부분 이유가 있다

기여할 이슈를 찾으면서 열린 이슈 200건을 GraphQL로 교차참조와 함께 받아 분류했습니다.

| 분류 | 건수 |
| --- | --- |
| 이미 열린 PR 있음 | 105건 (53%) |
| PR 전혀 없음 | 61건 |
| 머지된 PR만 있음(이미 수정됨) | 21건 |
| 닫힌 PR만 있음(시도 실패) | 13건 |

절반 이상이 선점 상태였고, 비어 있는 61건도 본문을 읽어보면 대부분 이유가 있었습니다. 메인테이너가 "버그가 아니다"라고 판단했거나, 신고자가 철회했거나, 설계가 미확정이거나, 신고자 본인이 착수 예정이었습니다. 실제로 `#65509`는 신고자가 "구현 전 방향 합의를 구한다"고 적어둔 상태였고, `#63305`은 jasnell이 "backpressure model is still an open question"이라고 답한 상태여서 둘 다 제외했습니다.

**교차참조만으로는 "PR 없음"까지만 알 수 있고, 임자·반대·철회는 스레드를 읽어야 드러납니다.** 자동 필터로 후보를 좁히되 최종 판단은 본문 정독이라는 절차를 세우게 되었습니다.

### 3. 빌드 트리는 브랜치를 따라오지 않는다

한 달 전 빌드해둔 트리에서 새 브랜치를 만들고 `ninja`를 돌렸더니 `child_process_permission.cc` 없음 오류가 났습니다. 그 사이 `src/permission/`이 리팩토링되어(파일 통합, `BooleanPermissions` 도입) 기존 `build.ninja`가 무효화된 것이었고, `./configure --ninja` 재실행이 필요했습니다.

또 하나, `make -j14 test`가 조용히 실패했습니다. 이 저장소의 Makefile은 `-j` 대신 `JOBS=14` 변수를 요구하는데, 잘못된 호출이 ninja에 빈 `-j`를 넘겨 즉시 죽었고 `tail`이 종료 코드를 삼켜 성공처럼 보였습니다. 출력 파일을 직접 열어보고서야 발견했습니다. **긴 명령의 결과는 종료 코드가 아니라 출력 내용으로 확인해야 한다**는 교훈입니다.

### 4. AI 사용 정책은 문서화되어 있다

`doc/contributing/ai-guidelines.md`가 공식 정책으로 존재합니다. AI 사용을 금지하지 않지만 **도구 사용 사실과 개인 검증 내용의 공개**를 요구하고, 특히 리뷰 피드백 응답의 AI 자동화를 명시적으로 금지합니다.

관례를 확인하기 위해 `git log --grep="Assisted-by"`를 돌려보니 랜딩 커밋이 198개 있었고, TSC 멤버들도 사용하고 있었습니다. 이 형식을 따라 커밋에 `Assisted-by` 트레일러를 남기고, PR 본문에 직접 수행한 검증(재현, 소스 추적, 테스트 실행)을 밝혔습니다. 리뷰 응답은 정책에 따라 직접 작성할 예정입니다.