Skip to content

fix(security): reject malformed artifact-token structures - #276

Draft
seonghobae wants to merge 7 commits into
fix/pii-logging-16240128950440010639from
fix/artifact-token-boundaries-2ff43f
Draft

fix(security): reject malformed artifact-token structures#276
seonghobae wants to merge 7 commits into
fix/pii-logging-16240128950440010639from
fix/artifact-token-boundaries-2ff43f

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Security objective

Reject malformed signed artifact-token structures before authorization or token-ledger access while preserving exact HMAC-bound payload semantics.

Exact current stack

The branch is not yet based on the declared parent. PR #288 restores the full parent, including the canonical unittest stacked-PR CI regression, while preserving this signed-token hardening slice. No result from #274, predecessor heads, or pre-reconciliation revisions transfers.

Boundary contract

  • Preserve trailing empty fields through exact splitting so surplus delimiters fail the field-count gate.
  • Reject null, empty, and whitespace-only signed text claims without trimming or normalizing the verified payload.
  • Reject malformed Base64URL, epoch, UUID, version, delimiter, field-count, and signature cases before authorization or token-ledger access.
  • Retain deterministic valid-structure, wrong-signature, empty-claim, correctly signed whitespace-only-claim, and fuzz regressions.
  • Keep release evidence free of unsupported performance claims.

Current remediation

CodeRabbit correctly identified that the prior deterministic tests covered an empty required text claim but not a correctly signed whitespace-only claim. Exact head 801e0ab5ce81fea10d331db7ddd089244a4171f2 adds rejectsSignedPayloadWithAWhitespaceOnlyRequiredField, pins it in scripts/test_artifact_token_parser_evidence.py, and records the fail-closed behavior in CHANGELOG.md.

Evidence boundary

The exact-head CI run has a transient GitHub Actions setup failure resolving pinned actions, while its merge-compatibility job succeeded; remaining exact-head Maven and fuzz work is queued. This is not passing and predecessor results do not transfer. Merge order remains #288 reconciliation, reinspection of the resulting exact head, then every required exact-head/base-sensitive CI, fuzz, security, review, approval, and repository-policy gate. Keep this PR Draft.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2179b97d-e755-4ef8-abe5-be82c98a10b8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@opencode-agent @cwl-noema-review Independently review exact current head b80800d57a419c70c1b386df6d665dab57df9eb5 only. This is a clean two-commit test-first reconstruction directly on parent #270 exact head 2ff43fc1b280f6de2ca903695e5cbeef5c0b3c17, exactly 2 commits ahead and 0 behind with five changed files. Verify malformed-token field-count handling, trailing-empty preservation, nonblank signed-claim construction without normalization, early failure before ledger access, deterministic boundary regressions, retained fuzzing, and absence of unsupported performance claims. Current fuzz is in progress; CI, Security Scan, SAST, Strix, formal exact-head reviews, and counted independent approval are absent or pending and are not passing. Treat every #274 or predecessor-parent result as stale.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='b80800d57a419c70c1b386df6d665dab57df9eb5'
parent='2ff43fc1b280f6de2ca903695e5cbeef5c0b3c17'

printf '%s\n' '== Exact commits and ancestry =='
git show -s --format='%H%n%P%n%s' "$parent" "$head"
git merge-base --is-ancestor "$parent" "$head"
printf 'parent_is_ancestor=%s\n' "$?"
printf 'commits_ahead=%s\n' "$(git rev-list --count "${parent}..${head}")"
printf 'commits_behind=%s\n' "$(git rev-list --count "${head}..${parent}")"

printf '%s\n' '== Parent-relative changed files =='
git diff --name-status "${parent}...${head}"

printf '%s\n' '== Parent-relative diff =='
git diff --find-renames --find-copies "${parent}...${head}" -- \
  CHANGELOG.md \
  scripts/test_artifact_token_parser_evidence.py \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactTokenClaims.java \
  src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java

printf '%s\n' '== Java structure maps =='
ast-grep outline src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java --items all
ast-grep outline src/main/java/com/clearfolio/viewer/artifact/ArtifactTokenClaims.java --items all
ast-grep outline src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java --items all

printf '%s\n' '== Relevant token and ledger call contexts =='
rg -n -C 5 -P '\b(split|ArtifactTokenClaims|tokenLedger|ledger|verify|parse|Base64|Hmac|HMAC)\b' \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactTokenClaims.java \
  src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java

printf '%s\n' '== Fuzz-target references and performance-claim scan =='
rg -n -i -C 2 -P '\b(fuzz|jqf|quickcheck|property|latency|throughput|allocation|performan(?:ce|t))\b' \
  CHANGELOG.md scripts src/main/java src/test/java || true

Length of output: 51793


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Verification and parsing flow =='
sed -n '185,235p' src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java
sed -n '335,372p' src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java

printf '%s\n' '== Ledger interface and all service ledger invocations =='
fd -a '^ArtifactLinkLedger\.java$|^InMemoryArtifactLinkLedger\.java$' src
rg -n -C 4 -P '\bartifactLinkLedger\.[A-Za-z_][A-Za-z0-9_]*\s*\(' \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java
rg -n -C 5 -P '\b(interface|class)\s+(ArtifactLinkLedger|InMemoryArtifactLinkLedger)\b|\b(find|resolve|lookup|recordRead|revoke)\s*\(' \
  src/main/java/com/clearfolio/viewer/artifact

printf '%s\n' '== Boundary-test token construction and assertions =='
sed -n '45,170p' src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java

printf '%s\n' '== Retained artifact-token fuzz target =='
ast-grep outline src/test/java/com/clearfolio/viewer/fuzz/ArtifactTokenParserFuzzTest.java --items all
sed -n '1,240p' src/test/java/com/clearfolio/viewer/fuzz/ArtifactTokenParserFuzzTest.java

printf '%s\n' '== Commit-by-commit changed-file scope =='
git show --format='commit %H%n%s' --name-status --no-renames 3a99b1c5114353911c77c3cd8ef4365a5cf9db14
git show --format='commit %H%n%s' --name-status --no-renames b80800d57a419c70c1b386df6d665dab57df9eb5

printf '%s\n' '== Existing tests for signature mismatch and delimiter-free inputs =='
rg -n -C 4 -i -P '\b(signature|mismatch|delimiter|malformed|invalid)\b' \
  src/test/java/com/clearfolio/viewer/artifact \
  src/test/java/com/clearfolio/viewer/fuzz/ArtifactTokenParserFuzzTest.java

Length of output: 31963


@seonghobae I completed the static review of exact head b80800d57a419c70c1b386df6d665dab57df9eb5.

I found one issue.

  • ArtifactTokenBoundaryTest does not add deterministic coverage for a delimiter-free malformed token or a signature mismatch. The stated boundary contract requires both cases. Add separate tests that assert 401 Unauthorized for each case.

The implementation otherwise preserves trailing empty fields, applies the field-count gate before HMAC verification and ledger lookup, and rejects blank signed text claims without normalization. The artifact-token fuzz target remains present.

I did not approve the PR. CI, fuzz execution, and the required external security gates remain pending.

You are interacting with an AI system.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@opencode-agent @cwl-noema-review Please independently review exact current head d82d47a916ea1a8a653d4b0e6a587eb60f33a8a8 relative to parent #270 exact 2ff43fc1b280f6de2ca903695e5cbeef5c0b3c17. Treat #274 and all predecessor-head results as stale.

The prior exact-head review identified missing deterministic coverage for delimiter-free malformed input and signature mismatch. Commit d82d47a916ea1a8a653d4b0e6a587eb60f33a8a8 adds separate 401 Unauthorized regressions for a Base64URL delimiter-free token and a structurally valid token carrying an HMAC of a different payload.

Recheck the five-file boundary slice: exact field count, trailing-empty preservation, nonblank signed claims without trimming, early rejection before authorization/ledger access, deterministic malformed Base64/epoch/UUID/version/signature cases, retained fuzz target, no unsupported performance claims, and preservation of all parent authorization, privacy, dependency, SBOM, report, coverage, and Javadoc gates.

Report exact-head findings or a no-blocker advisory conclusion after independent inspection. Keep Draft and do not infer counted independent approval.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='d82d47a916ea1a8a653d4b0e6a587eb60f33a8a8'
parent='2ff43fc1b280f6de2ca903695e5cbeef5c0b3c3c17'
# Correct the parent SHA from the requested review context.
parent='2ff43fc1b280f6de2ca903695e5cbeef5c0b3c17'

printf '%s\n' '== Exact ancestry and bounded change scope =='
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$parent" "$head"
git merge-base --is-ancestor "$parent" "$head"
printf 'parent_is_ancestor=%s\n' "$?"
printf 'ahead=%s behind=%s\n' \
  "$(git rev-list --count "${parent}..${head}")" \
  "$(git rev-list --count "${head}..${parent}")"
git diff --name-status "${parent}...${head}"
printf '%s\n' '== Parent-relative diff =='
git diff --find-renames --find-copies "${parent}...${head}" -- \
  CHANGELOG.md \
  scripts/test_artifact_token_parser_evidence.py \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactTokenClaims.java \
  src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java

printf '%s\n' '== Artifact parser and claim structure =='
ast-grep outline src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java --items all
ast-grep outline src/main/java/com/clearfolio/viewer/artifact/ArtifactTokenClaims.java --items all
ast-grep outline src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java --items all
printf '%s\n' '== Parser, authorization, and ledger call contexts =='
rg -n -C 8 -P '\b(split|parse|verify|Hmac|HMAC|Base64|ArtifactTokenClaims|artifactLinkLedger|authorize|authorization|ledger)\b' \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactTokenClaims.java

printf '%s\n' '== Deterministic boundary tests =='
sed -n '1,280p' src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java
printf '%s\n' '== Retained fuzz target =='
fd -a '^ArtifactTokenParserFuzzTest\.java$' src
fuzz_file="$(fd -a '^ArtifactTokenParserFuzzTest\.java$' src | head -n 1)"
if [ -n "$fuzz_file" ]; then
  ast-grep outline "$fuzz_file" --items all
  sed -n '1,260p' "$fuzz_file"
fi

printf '%s\n' '== Claims, performance statements, and gate-related changes in bounded slice =='
rg -n -i -C 3 -P '\b(latency|throughput|allocation|performan(?:ce|t)|fuzz|authorization|privacy|dependenc(?:y|ies)|SBOM|report|coverage|Javadoc)\b' \
  CHANGELOG.md scripts \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactTokenClaims.java \
  src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java || true

printf '%s\n' '== Exact-head PR state, checks, and required-review metadata =='
gh pr view 276 --repo ContextualWisdomLab/clearfolio \
  --json number,isDraft,headRefOid,baseRefOid,reviewDecision,reviews,statusCheckRollup \
  --jq '{number,isDraft,headRefOid,baseRefOid,reviewDecision,reviews:[.reviews[]|{author:.author.login,state}],checks:[.statusCheckRollup[]|{name,conclusion,status}]}'

Length of output: 108273


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='d82d47a916ea1a8a653d4b0e6a587eb60f33a8a8'
parent='2ff43fc1b280f6de2ca903695e5cbeef5c0b3c17'

printf '%s\n' '== Exact review-fix commit delta =='
git show --format='commit=%H%nparents=%P%nsubject=%s' --find-renames --find-copies "$head" -- \
  src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java \
  scripts/test_artifact_token_parser_evidence.py

printf '%s\n' '== Complete parser implementation =='
sed -n '335,370p' src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java

printf '%s\n' '== Parent gate files changed by this slice (must be empty) =='
git diff --name-status "${parent}...${head}" -- \
  .github \
  pom.xml \
  '**/pom.xml' \
  docs \
  qa || true

printf '%s\n' '== Workflow and build-test discovery configuration =='
fd -a -t f '^(pom\.xml|.*\.(yml|yaml))$' .github . 2>/dev/null | sort | while IFS= read -r file; do
  printf '%s\n' "--- $file"
  rg -n -i -C 3 -P 'test_artifact_token_parser_evidence|pytest|unittest|python3 scripts|mvn[[:space:]].*(test|verify)|javadoc|cyclonedx|sbom|coverage|security|sast|fuzz' "$file" || true
done

printf '%s\n' '== References to the new evidence test =='
rg -n -C 3 -F 'test_artifact_token_parser_evidence' . || true

printf '%s\n' '== Existing ledger-access boundary tests or test doubles =='
rg -n -C 4 -P '(findByTokenId|ArtifactLinkLedger|ledger).*(assert|verify|throw|count)|((assert|verify|count).*(findByTokenId|ArtifactLinkLedger|ledger))' \
  src/test/java/com/clearfolio/viewer/artifact || true

printf '%s\n' '== Whitespace-claim regressions =='
rg -n -C 3 -P '(isBlank|blank|required field|\"[[:space:]]+\")' \
  src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java \
  src/test/java/com/clearfolio/viewer/fuzz/ArtifactTokenParserFuzzTest.java || true

printf '%s\n' '== Exact-head commit checks, reviews, and branch protection metadata =='
gh api repos/ContextualWisdomLab/clearfolio/commits/"$head"/check-runs \
  -H 'Accept: application/vnd.github+json' \
  --jq '{total_count,check_runs:[.check_runs[]|{name,status,conclusion,details_url}]}'
gh api repos/ContextualWisdomLab/clearfolio/branches/main/protection \
  -H 'Accept: application/vnd.github+json' \
  --jq '{required_status_checks,required_pull_request_reviews,required_linear_history,allow_force_pushes,allow_deletions}' || true

Length of output: 24082


@seonghobae I completed an independent static review of exact head d82d47a916ea1a8a653d4b0e6a587eb60f33a8a8 against 2ff43fc1b280f6de2ca903695e5cbeef5c0b3c17.

No blocker found in the five-file boundary slice.

  • The parent is an ancestor of this head. The branch is three commits ahead and zero commits behind.
  • parseAndVerify uses split("\\.", -1). It rejects any count other than ten payload fields plus one signature.
  • The field-count rejection occurs before HMAC verification. HMAC verification occurs before claim decoding and construction.
  • verifyReadToken calls parseAndVerify before artifactLinkLedger.findByTokenId. Malformed tokens therefore fail before token-ledger access.
  • ArtifactTokenClaims rejects null and blank text claims with Objects.requireNonNull and String.isBlank. It does not trim or normalize signed claim values.
  • ArtifactTokenBoundaryTest contains deterministic 401 Unauthorized coverage for nine and eleven fields, a trailing delimiter, delimiter-free input, mismatched HMAC, empty claim, malformed Base64URL, invalid epoch values, malformed UUID, and unsupported version.
  • ArtifactTokenParserFuzzTest remains present and is still selected by .github/workflows/fuzz.yml.
  • The slice does not modify workflow, dependency, SBOM, report, coverage, privacy, authorization, or Javadoc gate configuration.
  • I found no new unsupported allocation, latency, throughput, or regex-performance claim in the boundary changes.

The three exact-head fuzz checks completed successfully, including fuzz (ArtifactTokenParserFuzzTest).

PR #276 remains Draft. No reviews are recorded. I could not verify main branch-protection requirements because the GitHub integration returned HTTP 403. I did not infer a counted independent approval or merge readiness.

You are interacting with an AI system.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@opencode-agent @cwl-noema-review Independently review exact current head d82d47a916ea1a8a653d4b0e6a587eb60f33a8a8 only. It is exactly three commits ahead and zero behind parent #270 head 2ff43fc1b280f6de2ca903695e5cbeef5c0b3c17, with the same five-file bounded slice. The latest commit adds the two deterministic regressions requested on predecessor head b80800d57a419c70c1b386df6d665dab57df9eb5: delimiter-free malformed input and a structurally valid token with a mismatched signature, both requiring 401 Unauthorized. Verify early structural rejection before ledger access, trailing-empty preservation, nonblank exact signed claims without normalization, retained fuzz coverage, and absence of unsupported performance claims. CodeRabbit commit status is successful, but no formal exact-head review exists; CI, Security Scan, SAST, fuzz, Strix, OpenCode/Noema review, and counted independent approval are absent or pending and are not passing. Treat all predecessor-head evidence as stale.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='d82d47a916ea1a8a653d4b0e6a587eb60f33a8a8'
parent='2ff43fc1b280f6de2ca903695e5cbeef5c0b3c17'

printf '%s\n' '== Exact ancestry and bounded scope =='
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$parent" "$head"
git merge-base --is-ancestor "$parent" "$head"
printf 'parent_is_ancestor=%s\n' "$?"
printf 'ahead=%s behind=%s\n' \
  "$(git rev-list --count "${parent}..${head}")" \
  "$(git rev-list --count "${head}..${parent}")"
git diff --name-status "${parent}...${head}"

printf '%s\n' '== Parent-relative five-file diff =='
git diff --find-renames --find-copies "${parent}...${head}" -- \
  CHANGELOG.md \
  scripts/test_artifact_token_parser_evidence.py \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactTokenClaims.java \
  src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java

printf '%s\n' '== Relevant source and deterministic-test structure =='
ast-grep outline src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java --items all
ast-grep outline src/main/java/com/clearfolio/viewer/artifact/ArtifactTokenClaims.java --items all
ast-grep outline src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java --items all

printf '%s\n' '== Token parser, verification, authorization, and ledger contexts =='
rg -n -C 10 -P '\b(parseAndVerify|verifyReadToken|split|verify|Hmac|HMAC|Base64|ArtifactTokenClaims|artifactLinkLedger|findByTokenId|authorize)\b' \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactTokenClaims.java

printf '%s\n' '== Complete boundary-test file =='
sed -n '1,320p' src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java

printf '%s\n' '== Fuzz target discovery and source =='
fd -a -t f '^ArtifactTokenParserFuzzTest\.java$' src
fuzz_file="$(fd -a -t f '^ArtifactTokenParserFuzzTest\.java$' src | head -n 1)"
if [ -n "$fuzz_file" ]; then
  ast-grep outline "$fuzz_file" --items all
  sed -n '1,280p' "$fuzz_file"
fi

printf '%s\n' '== Claims and unsupported-performance-claim scan in bounded files =='
rg -n -i -C 3 -P '\b(latency|throughput|allocation|performan(?:ce|t)|regex|fuzz)\b' \
  CHANGELOG.md \
  scripts/test_artifact_token_parser_evidence.py \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactTokenClaims.java \
  src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java || true

printf '%s\n' '== Exact-head GitHub PR metadata and checks =='
gh pr view 276 --repo ContextualWisdomLab/clearfolio \
  --json isDraft,headRefOid,baseRefOid,reviewDecision,reviews,statusCheckRollup \
  --jq '{isDraft,headRefOid,baseRefOid,reviewDecision,reviews:[.reviews[]|{author:.author.login,state,commit_id:.commit.oid}],checks:[.statusCheckRollup[]|{name,status,conclusion,headSha:.detailsUrl}]}'

Length of output: 78323


@seonghobae I completed an independent static review of exact head d82d47a916ea1a8a653d4b0e6a587eb60f33a8a8 against parent 2ff43fc1b280f6de2ca903695e5cbeef5c0b3c17.

I found one blocker.

  • ArtifactTokenBoundaryTest tests an empty required claim ("") but does not test a whitespace-only required claim, such as " ". The production code correctly uses String.isBlank(). However, the deterministic regression does not protect that required behavior. A future replacement with String.isEmpty() would pass all current boundary tests and accept whitespace-only signed claims. Add a correctly signed token with a whitespace-only text claim and assert 401 Unauthorized.

The remaining requested checks passed by static inspection:

  • The parent is an ancestor of the reviewed head. The branch is three commits ahead and zero commits behind.
  • The parent-relative slice changes the stated five files.
  • parseAndVerify uses split("\\.", -1). It rejects invalid field counts before HMAC verification.
  • verifyReadToken calls parseAndVerify before artifactLinkLedger.findByTokenId.
  • ArtifactTokenClaims rejects null and blank text claims without trimming or normalizing the signed values.
  • The delimiter-free malformed-token and mismatched-signature regressions are present. Both assert 401 Unauthorized.
  • ArtifactTokenParserFuzzTest remains present.
  • I found no new unsupported allocation, latency, throughput, regex, or performance claim in the boundary changes.

GitHub currently reports successful exact-head fuzz checks, including fuzz (ArtifactTokenParserFuzzTest). PR #276 remains Draft. GitHub reports no formal reviews. I did not infer an independent approval or merge readiness.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae
seonghobae changed the base branch from fix/pii-logging-16240128950440010639 to snapshot/buyer-parent-2ff43fc August 6, 2026 13:09
@seonghobae
seonghobae changed the base branch from snapshot/buyer-parent-2ff43fc to fix/pii-logging-16240128950440010639 August 6, 2026 13:20

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@opencode-agent @cwl-noema-review Please independently review exact current head 89c4ebe1b294b9c429b736a252243cb60c985673 relative to exact parent #270 head c1239e20b048a582b19cb9eeb90f2c29f7c8f814. Do not transfer #274 or predecessor-head evidence.

Verify the signed artifact-token parser boundary: preserve trailing empty fields; reject surplus or missing delimiters and wrong field counts; reject null or blank signed text claims without trimming or normalizing the HMAC-verified payload; reject malformed Base64URL, version, epoch, UUID, and signature structures before authorization or token-ledger access; retain deterministic valid-structure/wrong-signature regressions and fuzz coverage; preserve exact HMAC semantics and fail-closed errors without sensitive reflection.

Exact-head CI 31105521510 succeeded, including Maven exact head, synthetic merge, buyer-readiness scripts, 100% production line/branch coverage, warning-free Javadocs, and parser-evidence tests. Exact-head fuzz 31105523110 succeeded. No unresolved inline threads exist. Security Scan, SAST Semgrep, Strix, formal exact-head reviews, and counted independent write-authorized approval remain absent and must not be inferred. Keep Draft; do not merge or publish.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='89c4ebe1b294b9c429b736a252243cb60c985673'
parent='c1239e20b048a582b19cb9eeb90f2c29f7c8f814'

printf '%s\n' '== Exact ancestry and change scope =='
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$parent" "$head"
git merge-base --is-ancestor "$parent" "$head"
printf 'parent_is_ancestor=%s\n' "$?"
printf 'ahead=%s behind=%s\n' \
  "$(git rev-list --count "${parent}..${head}")" \
  "$(git rev-list --count "${head}..${parent}")"
git diff --name-status "${parent}...${head}"

printf '%s\n' '== Parent-relative diff =='
git diff --find-renames --find-copies "${parent}...${head}"

printf '%s\n' '== Candidate source and test structure =='
fd -a -t f '^(ArtifactLinkService|ArtifactTokenClaims|ArtifactTokenBoundaryTest|ArtifactTokenParserFuzzTest)\.java$' src | sort | while IFS= read -r file; do
  printf '%s\n' "--- $file"
  ast-grep outline "$file" --items all
done

printf '%s\n' '== Parser, claims, authorization, and ledger call contexts =='
rg -n -C 10 -P '\b(parseAndVerify|verifyReadToken|split|HMAC|Hmac|Mac|Base64|ArtifactTokenClaims|artifactLinkLedger|findByTokenId|isBlank|trim|strip)\b' \
  src/main/java/com/clearfolio/viewer/artifact \
  src/test/java/com/clearfolio/viewer/artifact \
  src/test/java/com/clearfolio/viewer/fuzz

printf '%s\n' '== Complete relevant implementation and deterministic test files =='
for file in \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactTokenClaims.java \
  src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java \
  scripts/test_artifact_token_parser_evidence.py
do
  if [ -f "$file" ]; then
    printf '%s\n' "--- $file"
    cat -n "$file"
  fi
done

printf '%s\n' '== Retained fuzz target and workflow selection =='
fd -a -t f '^ArtifactTokenParserFuzzTest\.java$' src .github | sort | while IFS= read -r file; do
  printf '%s\n' "--- $file"
  cat -n "$file"
done
rg -n -C 4 -P 'ArtifactTokenParserFuzzTest|fuzz' .github CHANGELOG.md scripts src/test || true

printf '%s\n' '== Exact-head PR state, reviews, and checks =='
gh pr view 276 --repo ContextualWisdomLab/clearfolio \
  --json isDraft,headRefOid,baseRefOid,reviewDecision,reviews,statusCheckRollup \
  --jq '{isDraft,headRefOid,baseRefOid,reviewDecision,reviews:[.reviews[]|{author:.author.login,state,commit:.commit.oid}],checks:[.statusCheckRollup[]|{name,status,conclusion}]}'

Length of output: 3195


🏁 Script executed:

#!/bin/bash
set -u

head='89c4ebe1b294b9c429b736a252243cb60c985673'
declared_parent='c1239e20b048a582b19cb9eeb90f2c29f7c8f814'
left_parent='2ff43fc1b280f6de2ca903695e5cbeef5c0b3c17'
right_parent='d82d47a916ea1a8a653d4b0e6a587eb60f33a8a8'

printf '%s\n' '== Ancestry matrix and merge bases =='
for from in "$declared_parent" "$left_parent" "$right_parent"; do
  for to in "$head" "$left_parent" "$right_parent"; do
    if git merge-base --is-ancestor "$from" "$to"; then
      result=yes
    else
      result=no
    fi
    printf '%s ancestor-of %s: %s\n' "$from" "$to" "$result"
  done
done
for pair in \
  "$declared_parent $head" \
  "$declared_parent $left_parent" \
  "$declared_parent $right_parent" \
  "$left_parent $right_parent"
do
  set -- $pair
  printf 'merge-base(%s,%s)=' "$1" "$2"
  git merge-base "$1" "$2" || true
done

printf '%s\n' '== Merge-result delta from each direct parent =='
for parent in "$left_parent" "$right_parent"; do
  printf '%s\n' "--- $parent..$head"
  git diff --name-status "$parent" "$head"
  git diff --find-renames --find-copies "$parent" "$head" -- \
    CHANGELOG.md \
    scripts/test_artifact_token_parser_evidence.py \
    src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java \
    src/main/java/com/clearfolio/viewer/artifact/ArtifactTokenClaims.java \
    src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java
done

printf '%s\n' '== Parser and claim implementation =='
sed -n '180,245p' src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java
sed -n '330,390p' src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java
cat -n src/main/java/com/clearfolio/viewer/artifact/ArtifactTokenClaims.java

printf '%s\n' '== Complete deterministic boundary tests =='
cat -n src/test/java/com/clearfolio/viewer/artifact/ArtifactTokenBoundaryTest.java

printf '%s\n' '== Ledger interface, call sites, and fuzz target =='
fd -a -t f '^(ArtifactLinkLedger|InMemoryArtifactLinkLedger|ArtifactTokenParserFuzzTest)\.java$' src | sort
rg -n -C 6 -P '\bartifactLinkLedger\.[A-Za-z_][A-Za-z0-9_]*\s*\(|\b(parseAndVerify|verifyReadToken)\s*\(' \
  src/main/java/com/clearfolio/viewer/artifact
fuzz_file="$(fd -a -t f '^ArtifactTokenParserFuzzTest\.java$' src | head -n 1)"
if [ -n "$fuzz_file" ]; then
  cat -n "$fuzz_file"
fi

printf '%s\n' '== Evidence and performance-claim scan =='
cat -n scripts/test_artifact_token_parser_evidence.py
rg -n -i -C 3 -P '\b(latency|throughput|allocation|performan(?:ce|t)|regex)\b' \
  CHANGELOG.md \
  scripts/test_artifact_token_parser_evidence.py \
  src/main/java/com/clearfolio/viewer/artifact \
  src/test/java/com/clearfolio/viewer/artifact || true

printf '%s\n' '== Exact-head GitHub metadata =='
gh pr view 276 --repo ContextualWisdomLab/clearfolio \
  --json isDraft,headRefOid,baseRefOid,reviewDecision,reviews,statusCheckRollup \
  --jq '{isDraft,headRefOid,baseRefOid,reviewDecision,reviews:[.reviews[]|{author:.author.login,state,commit:.commit.oid}],checks:[.statusCheckRollup[]|{name,status,conclusion}]}' || true

Length of output: 59238


@seonghobae I found two blockers.

  1. The reviewed head is not based on the stated parent.
    c1239e20b048a582b19cb9eeb90f2c29f7c8f814 is not an ancestor of 89c4ebe1b294b9c429b736a252243cb60c985673. Their merge-base is 2ff43fc1b280f6de2ca903695e5cbeef5c0b3c17. The head is a merge commit of 2ff43fc1b280f6de2ca903695e5cbeef5c0b3c17 and d82d47a916ea1a8a653d4b0e6a587eb60f33a8a8. Rebase or otherwise reconcile the branch onto c1239e20b048a582b19cb9eeb90f2c29f7c8f814. Then regenerate the exact-head and base-sensitive evidence.

  2. ArtifactTokenBoundaryTest does not test a correctly signed whitespace-only required text claim.
    ArtifactTokenClaims.requireText correctly uses String.isBlank(). The test suite only covers "". A future change to String.isEmpty() would pass the current deterministic tests and accept " " as a signed claim. Add a token with a whitespace-only text field and assert 401 Unauthorized.

The parser implementation otherwise meets the inspected boundary requirements:

  • split("\\.", -1) preserves trailing empty fields.
  • The exact field-count gate runs before HMAC verification.
  • HMAC verification runs before Base64URL decoding and claim construction.
  • verifyReadToken calls parseAndVerify before artifactLinkLedger.findByTokenId.
  • Claim validation does not trim or normalize signed values.
  • The delimiter-free and mismatched-signature regressions are present.
  • The fuzz target remains present.
  • I found no new unsupported performance claim in the boundary slice.

GitHub reports successful Maven, synthetic-merge, buyer-readiness, and fuzz checks for this head. The PR remains Draft. No formal review or independent approval exists. I did not infer the absent security gates or merge readiness.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant