fix(core): protect canonical evidence runtime types - #63
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough감사 이벤트와 작업 분석 증거의 런타임 타입 검증을 강화했습니다. 내장 타입 서브클래스를 거부하고, 허용된 타임스탬프를 UTC로 고정합니다. 생성 후 변조와 변환 예외에 대한 회귀 테스트를 추가했습니다. Changes런타임 타입 무결성
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The runtime-type and timestamp hardening changes have focused regression coverage, and no current merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
seonghobae
changed the title
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent Please review the current unchanged head against protected |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@coderabbitai review Please review exact head |
|
@coderabbitai review Please review exact current head for #213 only. Focus on whether module import still exposes mutable creation/live issuance backing storage, whether the closure-private claim/record/lookup API can reset or overwrite an already-issued live identity, whether stale-finalizer identity protection remains intact, and whether #211/#212 canonicalization/re-entry semantics were preserved. Do not transfer predecessor-head findings or checks. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== exact HEAD and status ==='
git rev-parse HEAD
git status --short
git branch --show-current
echo '=== recent commits ==='
git log --oneline -12
echo '=== diff from previous reviewed successor ==='
git diff --stat 31fabb5764f4fc6e09884da83348eb0fb50ea5c5..HEAD
git diff --find-renames --unified=80 31fabb5764f4fc6e09884da83348eb0fb50ea5c5..HEAD -- \
packages/hris-kernel/src/orgmetra_hris_kernel/audit.py \
packages/hris-kernel/tests \
manifest.json \
CHANGELOG.md
echo '=== audit module structure ==='
ast-grep outline packages/hris-kernel/src/orgmetra_hris_kernel/audit.py --items all
echo '=== focused source ==='
cat -n packages/hris-kernel/src/orgmetra_hris_kernel/audit.py
echo '=== audit-related test definitions and focused content ==='
rg -n --type py -C 3 \
'(_AUDIT_CREATION_SNAPSHOTS|_AUDIT_LIVE_ISSUANCES|_claim|_record|_lookup|_cleanup|finaliz|__post_init__|to_cloudevent|canonical_timestamp|creation.snapshot|re.?entr)' \
packages/hris-kernel/tests
echo '=== package exports and direct registry imports ==='
fd -a '__init__.py' packages/hris-kernel/src -x sh -c 'echo === $1; cat -n "$1"' sh {}
rg -n --type py -C 3 \
'(from\s+orgmetra_hris_kernel\.audit\s+import|import\s+orgmetra_hris_kernel\.audit|_AUDIT_CREATION_SNAPSHOTS|_AUDIT_LIVE_ISSUANCES)' \
packages/hris-kernel
echo '=== manifest rule and current entries ==='
rg -n -C 4 '(REQUIRED|_validate_manifest|audit\.py|test_audit_outbox\.py)' tests/validate_repository.py manifest.jsonLength of output: 201723
Blocking — closure cells still expose mutable issuance storage. After a consumer removes the live issuance for an existing event, it can mutate event fields with The new test checks only that The normal paths preserve the requested semantics:
However, these properties do not prevent the closure-state reset and reissue path. You are interacting with an AI system. |
|
@coderabbitai review Please review exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base="eb9757f8649aaad026a9865508d9aad50c1a7a4f"
head="72070cb4b8d636825ce5b1a326df4c296596ed7e"
echo "=== exact revision ==="
git rev-parse HEAD
git status --short
git merge-base "$base" "$head"
git diff --stat "$base..$head"
git diff --name-status "$base..$head"
echo "=== final successor delta ==="
git log --oneline --decorate -12
parent="$(git rev-parse "${head}^")"
git diff --stat "$parent..$head"
git diff --name-status "$parent..$head"
git diff --find-renames --unified=60 "$parent..$head"
echo "=== audit module outline ==="
ast-grep outline packages/hris-kernel/src/orgmetra_hris_kernel/audit.py --items all
echo "=== audit branch neighborhoods ==="
nl -ba packages/hris-kernel/src/orgmetra_hris_kernel/audit.py | sed -n '55,82p;175,205p;222,250p'
echo "=== test definitions and changed test content ==="
ast-grep outline packages/hris-kernel/tests --items all --type function --match 'audit|Audit'
git diff --find-renames --unified=80 "$parent..$head" -- packages/hris-kernel/tests
rg -n --type py -C 5 \
'(Line 71|Line 192|Line 239|structural|reject|_validate_event_snapshot|AuditOutboxEvent|to_cloudevent)' \
packages/hris-kernel/tests
echo "=== production and manifest contract changes at exact head ==="
git diff --find-renames --unified=40 "$parent..$head" -- \
packages/hris-kernel/src/orgmetra_hris_kernel/audit.py \
manifest.json \
tests/validate_repository.py \
CHANGELOG.md
rg -n -C 5 '(REQUIRED|_validate_manifest|audit\.py|test_audit_outbox\.py)' \
tests/validate_repository.py manifest.jsonLength of output: 13352 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="eb9757f8649aaad026a9865508d9aad50c1a7a4f"
head="72070cb4b8d636825ce5b1a326df4c296596ed7e"
parent="$(git rev-parse "${head}^")"
echo "=== requested branch neighborhoods, current source ==="
sed -n '45,90p;145,255p' packages/hris-kernel/src/orgmetra_hris_kernel/audit.py
echo "=== full canonical export path, current source ==="
sed -n '250,390p' packages/hris-kernel/src/orgmetra_hris_kernel/audit.py
echo "=== exact final test file ==="
cat packages/hris-kernel/tests/test_audit_runtime_type_integrity.py
echo "=== prior test file at final parent ==="
git show "${parent}:packages/hris-kernel/tests/test_audit_runtime_type_integrity.py"
echo "=== source and manifest-rule changes from protected base ==="
git diff --find-renames --unified=25 "$base..$head" -- \
packages/hris-kernel/src/orgmetra_hris_kernel/audit.py \
tests/validate_repository.py \
manifest.json
echo "=== manifest validation contract and relevant final entries ==="
sed -n '/REQUIRED/,/^[^[:space:]]/p' tests/validate_repository.py
rg -n -C 4 '"(CHANGELOG\.md|packages/hris-kernel/src/orgmetra_hris_kernel/audit\.py|packages/hris-kernel/tests/test_audit_outbox\.py)"' manifest.json
echo "=== static test imports and collection names ==="
rg -n --type py -C 2 \
'^(class _ForgedAuditOutboxEvent|def test_audit_event_rejects_(subclass_construction|low_level_subclass|low_level_non_utc_datetime)|from orgmetra_hris_kernel\.audit import AuditOutboxEvent)' \
packages/hris-kernel/tests/test_audit_runtime_type_integrity.py
echo "=== final commit metadata ==="
git show --format=fuller --no-ext-diff --no-renames --stat "$head"Length of output: 53109 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== exact source line mapping ==="
awk 'NR >= 64 && NR <= 78 { printf "%d:%s\n", NR, $0 }
NR >= 184 && NR <= 198 { printf "%d:%s\n", NR, $0 }
NR >= 232 && NR <= 246 { printf "%d:%s\n", NR, $0 }' \
packages/hris-kernel/src/orgmetra_hris_kernel/audit.py
echo "=== manifest validator entry-set enforcement ==="
rg -n -A45 -B8 \
'def _manifest_entries|def _validate_manifest|unexpected|extra|set\(REQUIRED\)|manifest.*entries' \
tests/validate_repository.py
echo "=== manifest metadata versus checked-in required artifacts ==="
for file in \
CHANGELOG.md \
packages/hris-kernel/src/orgmetra_hris_kernel/audit.py \
packages/hris-kernel/tests/test_audit_outbox.py
do
printf '%s\t' "$file"
sha256sum "$file" | awk '{printf "sha256=%s ", $1}'
bytes=$(wc -c < "$file")
lines=$(awk 'END { print NR }' "$file")
printf 'bytes=%s lines=%s\n' "$bytes" "$lines"
done
echo "=== final commit does not modify production or manifest paths ==="
git diff-tree --no-commit-id --name-status -r 72070cb4b8d636825ce5b1a326df4c296596ed7e
git diff --quiet 72070cb4b8d636825ce5b1a326df4c296596ed7e^ 72070cb4b8d636825ce5b1a326df4c296596ed7e -- \
packages/hris-kernel/src/orgmetra_hris_kernel/audit.py manifest.json tests/validate_repository.py
printf 'production_and_manifest_paths_unchanged_in_final_commit=%s\n' "$?"Length of output: 13724
The final commit changes only The three new regressions cover the specified structural rejection branches in
The final commit does not modify production code, I used the existing manifest-contract guidance. The validator still compares the manifest path set with the exact The current manifest metadata matches the checked-in required artifacts, including 🧠 Learnings usedYou are interacting with an AI system. |
|
@coderabbitai review Please review exact current head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base="eb9757f8649aaad026a9865508d9aad50c1a7a4f"
head="72070cb4b8d636825ce5b1a326df4c296596ed7e"
echo "=== revision identity ==="
printf 'HEAD='
git rev-parse HEAD
printf 'requested_head=%s\n' "$head"
printf 'merge_base='
git merge-base "$base" "$head"
git status --short
echo "=== final successor paths and diff ==="
parent="$(git rev-parse "${head}^")"
git diff-tree --no-commit-id --name-status -r "$head"
git diff --check "$parent..$head"
git diff --find-renames --unified=80 "$parent..$head" -- \
packages/hris-kernel/tests/test_audit_runtime_type_integrity.py \
packages/hris-kernel/src/orgmetra_hris_kernel/audit.py \
manifest.json \
tests/validate_repository.py \
CHANGELOG.md
echo "=== final test structure ==="
ast-grep outline packages/hris-kernel/tests/test_audit_runtime_type_integrity.py --items all --type function,class
nl -ba packages/hris-kernel/tests/test_audit_runtime_type_integrity.py | sed -n '1,280p'
echo "=== corresponding audit branches and export path ==="
ast-grep outline packages/hris-kernel/src/orgmetra_hris_kernel/audit.py --items all
nl -ba packages/hris-kernel/src/orgmetra_hris_kernel/audit.py | sed -n '55,80p;180,200p;230,248p;250,390p'
echo "=== protected-base changes for production and manifest-owned paths ==="
git diff --find-renames --unified=25 "$base..$head" -- \
packages/hris-kernel/src/orgmetra_hris_kernel/audit.py \
manifest.json \
tests/validate_repository.py \
CHANGELOG.md
echo "=== manifest contract and current required artifact metadata ==="
rg -n -A55 -B8 \
'REQUIRED|def _validate_manifest|unexpected|extra|set\(REQUIRED\)' \
tests/validate_repository.py
rg -n -C 4 \
'"(CHANGELOG\.md|packages/hris-kernel/src/orgmetra_hris_kernel/audit\.py|packages/hris-kernel/tests/test_audit_outbox\.py)"' \
manifest.json
for file in \
CHANGELOG.md \
packages/hris-kernel/src/orgmetra_hris_kernel/audit.py \
packages/hris-kernel/tests/test_audit_outbox.py
do
printf '%s\t' "$file"
sha256sum "$file" | awk '{printf "sha256=%s ", $1}'
printf 'bytes=%s lines=%s\n' "$(wc -c < "$file")" "$(awk 'END {print NR}' "$file")"
done
echo "=== direct module-state exposure scan ==="
rg -n --type py -C 3 \
'(_AUDIT_CREATION_SNAPSHOTS|_AUDIT_LIVE_ISSUANCES|__closure__|_claim_audit_issuance|_record_audit_creation_snapshot|_lookup_audit_creation_snapshot)' \
packages/hris-kernel/src/orgmetra_hris_kernel/audit.py \
packages/hris-kernel/testsLength of output: 12502 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="eb9757f8649aaad026a9865508d9aad50c1a7a4f"
head="72070cb4b8d636825ce5b1a326df4c296596ed7e"
show_lines() {
local file="$1"
local ranges="$2"
awk -v ranges="$ranges" '
BEGIN {
count = split(ranges, parts, ";")
for (i = 1; i <= count; i++) {
split(parts[i], bounds, "-")
start[i] = bounds[1]
stop[i] = bounds[2]
}
}
{
for (i = 1; i <= count; i++) {
if (NR >= start[i] && NR <= stop[i]) {
printf "%d:%s\n", NR, $0
break
}
}
}
' "$file"
}
echo "=== source branch neighborhoods ==="
show_lines packages/hris-kernel/src/orgmetra_hris_kernel/audit.py "55-80;180-200;230-248;250-390"
echo "=== final test lines ==="
show_lines packages/hris-kernel/tests/test_audit_runtime_type_integrity.py "160-205"
echo "=== base-to-head production and manifest changes ==="
git diff --name-status "$base..$head" -- \
packages/hris-kernel/src/orgmetra_hris_kernel/audit.py \
manifest.json \
tests/validate_repository.py \
CHANGELOG.md
git diff --find-renames --unified=30 "$base..$head" -- \
packages/hris-kernel/src/orgmetra_hris_kernel/audit.py \
manifest.json \
tests/validate_repository.py \
CHANGELOG.md
echo "=== manifest validator contract ==="
rg -n -A55 -B8 \
'REQUIRED|def _validate_manifest|unexpected|extra|set\(REQUIRED\)' \
tests/validate_repository.py
echo "=== current manifest entries ==="
rg -n -C 4 \
'"(CHANGELOG\.md|packages/hris-kernel/src/orgmetra_hris_kernel/audit\.py|packages/hris-kernel/tests/test_audit_outbox\.py)"' \
manifest.json
echo "=== metadata compared with worktree ==="
for file in \
CHANGELOG.md \
packages/hris-kernel/src/orgmetra_hris_kernel/audit.py \
packages/hris-kernel/tests/test_audit_outbox.py
do
printf '%s\t' "$file"
sha256sum "$file" | awk '{printf "sha256=%s ", $1}'
printf 'bytes=%s lines=%s\n' "$(wc -c < "$file")" "$(awk 'END {print NR}' "$file")"
done
echo "=== audit module state exposure and lifecycle implementation ==="
rg -n --type py -C 5 \
'(_AUDIT_CREATION_SNAPSHOTS|_AUDIT_LIVE_ISSUANCES|__closure__|_claim_audit_issuance|_record_audit_creation_snapshot|_lookup_audit_creation_snapshot|finalize|cleanup)' \
packages/hris-kernel/src/orgmetra_hris_kernel/audit.py \
packages/hris-kernel/tests
echo "=== worktree test and final commit status ==="
git status --short
git diff --check "${head}^..$head"Length of output: 61026
The checked-out The final commit changes only
The tests target the rejection branches before untrusted subtype behavior or non-UTC timestamp rendering can proceed. No production file or manifest-owned artifact changes in the final successor. The current The previous mutable issuance-registry concern remains resolved on this head. This review does not determine hosted Foundation status or merge readiness from the separate central CodeQL dispatch handoff. 🧠 Learnings usedYou are interacting with an AI system. |
1 participant
Scope
Canonical shared HRIS-kernel audit/runtime evidence lane for Orgmetra. It owns shared audit/job-analysis value integrity; consumer lanes do not copy mutable #63 source.
Protected workflow adoption
Repository workflow owner #161 integrated normally as protected
develop@eb9757f8649aaad026a9865508d9aad50c1a7a4f. #63 adopted that protected truth without force/rebase through ordinary successors. Current exact head is72070cb4b8d636825ce5b1a326df4c296596ed7e, open · Draft · mechanically mergeable over protected develop.#211–#213 retained structural repair
#211 moved canonical timestamp validation ahead of snapshot equality so caller-defined timezone behavior cannot execute during low-level comparison. #212 repaired the then-current single-use audit issuance identity. #213 subsequently removed process-local mutable issuance authority entirely and made
AuditOutboxEventan exact-type, slotless tuple value object whose construction validates fields and detaches time evidence; export revalidates contained evidence. Low-level tuple construction is treated only as adversarial interpreter-level test input and must fail closed at export, not as a supported issuance path.#214 timezone-provider boundary
At predecessor
e29d180…,_freeze_timestamp(...)still invokedutcoffset()on arbitrary caller-suppliedtzinfoafter only outer-datetimevalidation. Ordinary successors1d24cef77d8b843f694c5982c5e4ceaf786eb864→7216153d55e2767716357d3171030d4aa8a64586→c9bbbef9740f6349930ff5ab79d39d0902f8a102→48af3c30dcf35724c719bac7c5f6202b3c2b4a0c→16dce30fea69991277c558688da704554c08b0e8established the exact standard-library provider contract: exact built-indatetime, then exactdatetime.timezoneorzoneinfo.ZoneInfo, before offset resolution; custom/executable timezone providers fail before callbacks. The manifest reseal covered only manifest-owned changed artifacts.Focused predecessor extraction had 39 manifest-owned audit tests passing and a direct
ZoneInfo("Asia/Seoul")/callback-tripwire probe. That local evidence remains causal only.Hosted RED RCA and minimal causal fix
Foundation CI
33904960273eventually acquired GitHub-hosted runner1001672934on the intendedubuntu-24.04image and checked out exact predecessor16dce30fea69991277c558688da704554c08b0e8. Foundation validation, runner contract, dependency hygiene, and all 218 HRIS-kernel tests passed, but the non-negotiable 100% coverage gate failed:audit.pyreported 116 statements / 3 misses, 60 branches / 3 partial, 97% file coverage; repository HRIS-kernel total was 99.43%. The exact uncovered rejection lines were 71, 192, and 239. PostgreSQL contracts were skipped only because that earlier step failed.The three uncovered branches are legitimate structural rejection paths rather than dead code:
_canonical_timestamp(...)rejects a low-level exact tuple carrying non-canonical time;AuditOutboxEvent.__new__()rejects subclass construction; andto_cloudevent()rejects a low-level tuple subclass before export. Lowering coverage or deleting these protections would weaken the contract.Ordinary test-only successor
72070cb4b8d636825ce5b1a326df4c296596ed7eadds exactly three regressions inpackages/hris-kernel/tests/test_audit_runtime_type_integrity.pyfor those hosted misses. It makes no production or manifest-owned-file change.Current exact-head evidence
Exact Foundation CI
33936088421completed successfully on current head72070cb4b8d636825ce5b1a326df4c296596ed7e. Repository-quality job101224196516checked out the exact candidate on GitHub-hostedubuntu-24.04; compile, foundation validation, dependency hygiene, owned unit/service-contract coverage, isolated PostgreSQL contracts, and read-only validation all completed successfully. The earlier 99.43% coverage RED is therefore repaired without weakening the 100% gate.Fresh exact-head security evidence has also completed: Security Scan
33936088403is success and SAST Semgrep33936088456is success. CodeQL PR33936088409is completed failure, but current job/log RCA does not show a SARIF/code vulnerability verdict.CodeQL compatibility analysis (actions)job101250523794and the Python shard entered the central exact-head handoff with no authenticatedcodeql-dispatch/<language>status, successfully requested their one-languagerepository_dispatch, recordedverdict=pending, then intentionally failed the required shard to release its runner. The job log states that the central dispatch workflow will publish the terminal verdict and rerun that exact failed job. Current commit statuses still have no terminalcodeql-dispatch/actionsorcodeql-dispatch/pythonverdict, so CodeQL remains non-passing; this failure is not converted into GREEN evidence.The canonical central owner already contains the dispatch/wake mechanics: ContextualWisdomLab/.github #1865 merged the exact failed-job wake design, and #1888 subsequently merged per-language dispatch concurrency so sibling language shards do not cancel each other. Current central
codeql-scan-dispatch.ymlvalidates live PR/base/head/run/job/language identity, executes the native CodeQL scan and SARIF Medium+ gate, publishescodeql-dispatch/<language>, then wakes only the matching failed required job. Fresh organization repository-dispatch evidence remains queued, so no no-op push or new competing leaf workaround is justified while the protected central owner is still executing its intended handoff.All visible review threads are resolved. Submitted reviews remain
COMMENTED; there is no ruleset-qualifying current-headAPPROVEDreview. #63 therefore remains Draft and is not merge-authorized despite Foundation/Security/SAST GREEN.A prior CodeRabbit suggestion to add Job Analysis files to
manifest.jsonwas verified against repository manifest equality rules and rejected/resolved rather than weakening the manifest contract.Owner dependencies
#64 remains the canonical generic People mutation writer. #65 owns purpose-bound authorization/Job Analysis consumer semantics and may consume shared-kernel truth only after #63 reaches protected
develop; fresh #64/#65 compare also requires #65 to preserve #64's protected consumer hardening before #65 integration. #163/#165 remain downstream of normally integrated protected owner truth.docs/product-technical-gap-baseline.mdremains single-writer-owned by #100.No self-approval, administrator bypass, gate weakening, force-push, destructive rebase, no-op retrigger, predecessor-check transfer, mutable-owner dependency, or source copy.