Skip to content

fix(kpi): harden collector output authority - #521

Merged
seonghobae merged 62 commits into
mainfrom
fix/kpi-collector-output-authority
Sep 2, 2026
Merged

fix(kpi): harden collector output authority#521
seonghobae merged 62 commits into
mainfrom
fix/kpi-collector-output-authority

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Scope

Harden KPI collection and provenance publication so buyer-facing operational evidence fails closed at network, pathname, inode, parent-authority, publication, descriptor-lifecycle, invocation-context, streaming, child-process lifecycle, source-identity rendering, and cross-process evidence-handoff boundaries. This branch remains Noema-only and does not synthesize production KPI evidence.

Current repair lineage

The branch owns exclusive no-follow log/provenance creation, parent and inode/version authority checks, bounded URL collection, close-failure handling, non-clobbering publication, permission restoration, helper resolution independent of caller CWD, realistic filesystem regressions, safe provenance source identifiers, descendant-aware timeout escalation, complete short-write handling, and stable provenance handoff after retained-log validation.

Historical review findings covering case-only aliases, special-file outputs, parent replacement, partial writes, late log mutation, close failures, background descendants, short writes, Unicode display controls, Windows Git Bash path conversion, and same-size mutation have been repaired on this lineage. Current review threads are resolved; predecessor evidence does not transfer to the current head.

Current protected-base convergence

Protected main advanced through runner-selector repair #532 to b505cb807536e9cf2bdbfcb05ad3fefe99999a14. The previous #521 head was 2 commits behind that protected tip. This lane was therefore non-destructively converged by a two-parent merge that preserves the existing KPI semantic delta while incorporating the exact current protected-main workflow/private-vulnerability-reporting changes.

Current exact head is 39f724a0401578dd1cbcd3cdd37a0e94b18f9ca6. Independent comparison against protected main@b505cb807536e9cf2bdbfcb05ad3fefe99999a14 reports ahead / 0 behind, with the protected main commit as the merge base and only the intended nine KPI/provenance paths differing from protected truth.

Fresh exact-head workflows were created after this convergence:

  • Security Scan 33551171591: queued / non-passing;
  • Application ci 33551171745: queued / non-passing; its current job has no assigned runner yet (runner_id=0, no executed steps);
  • reviewer-ci 33551171728: queued / non-passing;
  • patch-validator-image 33551171775: queued / non-passing.

No predecessor check, review, or artifact is reused after the head/base movement. The branch remains unmergeable-by-policy until the exact current head receives all applicable terminal-success CI/security/reviewer/image/SBOM/vulnerability/provenance evidence and live mergeability/governance are freshly unchanged.

Authentic >=30-day production KPI evidence remains an external operational acceptance item; fixtures and repository tests are not production evidence.

Guardrails

  • Do not weaken KPI integrity, exact coverage, scanner thresholds, image/SBOM/provenance, or branch governance.
  • Do not overwrite or auto-delete retained evidence when exact filesystem authority is uncertain.
  • Do not regress protected-main runtime/security contracts while converging this branch.
  • Do not merge after head/base/governance movement or while any applicable current-head gate is non-terminal.
  • Do not mutate central .github, naruon, contextual-orchestrator, or another dedicated-writer repository from this lane.

@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

Next included review available in 55 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 4fb9d148-dd18-455a-a41b-b16b43b9ab11

📥 Commits

Reviewing files that changed from the base of the PR and between 262bea1 and dd0a9ee.

📒 Files selected for processing (5)
  • scripts/collect-kpi-logs.sh
  • scripts/lib/private-no-replace-output.mjs
  • test/kpi-collector-output-authority.test.ts
  • test/kpi-source-id-control-characters.test.ts
  • test/private-no-replace-output.test.ts
📝 Walkthrough

Walkthrough

KPI 수집기는 로그와 provenance를 배타적 owner-only 파일로 생성합니다. parent chain, 파일 identity, source ID, 스트리밍 쓰기, deadline 및 실패 정리 동작을 검증합니다. 관련 테스트와 운영 문서를 갱신합니다.

Changes

KPI 출력 및 수집 검증 강화

Layer / File(s) Summary
비대체 출력 헬퍼와 실패 정리
scripts/lib/private-no-replace-output.mjs, test/private-no-replace-output.test.ts
출력 헬퍼가 `O_EXCL
수집기 스트리밍 및 provenance 처리
scripts/collect-kpi-logs.sh, docs/observability-kpi.md
수집기가 private 경로, 중복 경로, 심볼릭 링크 및 FIFO를 거부합니다. tail-command 결과를 비동기로 스트리밍하고 프로세스 그룹 종료와 600초 deadline을 처리합니다. 로그 안정성을 확인한 뒤 provenance를 게시합니다.
수집기 검증과 운영 기록
scripts/lib/source-id.mjs, test/kpi-collector-output-authority.test.ts, test/kpi-collector-production-url.test.ts, test/kpi-source-id-control-characters.test.ts, test/kpi-tail-streaming-integrity.test.ts, CHANGELOG.md
source_id의 제어문자와 Unicode formatting control을 거부합니다. 출력 경로, close 실패, 완전 쓰기, descendant 종료, curl timeout 및 저장 순서를 검증합니다. 변경 사항을 문서와 changelog에 기록합니다.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🟡 Moderate · up to 262be

This PR substantially hardens KPI evidence collection and source-label validation, but merge readiness remains moderate because symlinked invocation could select unauthorized helpers, log publication can expose evidence before its provenance is complete, and current-head checks are still queued; the identified path and source-label issues also require owner follow-up.

Sequence Diagram(s)

sequenceDiagram
  participant Collector as collect-kpi-logs.sh
  participant Source as curl 또는 tail command
  participant LogFile as 로그 파일
  participant Provenance as provenance 파일
  Collector->>LogFile: 배타적 no-follow 생성
  Collector->>Source: 열린 descriptor 전달
  Source-->>LogFile: KPI 로그 스트리밍
  Collector->>LogFile: descriptor와 parent chain 안정성 검증
  Collector->>Provenance: 비대체 provenance 게시
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.88% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 8 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 KPI collector의 output authority를 강화하는 핵심 변경을 정확히 설명합니다. PR의 다른 보안 강화 항목을 모두 포함하지 않지만, 제목은 간결하고 주요 변경과 관련됩니다.
Full details: Docstring Coverage

Explanation

Docstring coverage is 5.88% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 8 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch fix/kpi-collector-output-authority
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/kpi-collector-output-authority

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae marked this pull request as ready for review August 31, 2026 11:51
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

github-code-quality[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae seonghobae added bug Something isn't working priority: medium Normal-priority or P2 work status: needs-review type: bug Defect or incorrect behavior labels Sep 2, 2026 — with ChatGPT Codex Connector
@seonghobae
seonghobae merged commit 1a868c2 into main Sep 2, 2026
18 checks passed
@seonghobae
seonghobae deleted the fix/kpi-collector-output-authority branch September 2, 2026 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority: medium Normal-priority or P2 work status: needs-review type: bug Defect or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant