Skip to content

Flag context-padding scan evasion#2490

Open
vyctorbrzezowski wants to merge 4 commits into
openclaw:mainfrom
vyctorbrzezowski:brzezowski/context-padding-static-scan
Open

Flag context-padding scan evasion#2490
vyctorbrzezowski wants to merge 4 commits into
openclaw:mainfrom
vyctorbrzezowski:brzezowski/context-padding-static-scan

Conversation

@vyctorbrzezowski

@vyctorbrzezowski vyctorbrzezowski commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR adds a deterministic static scan signal for extreme context padding in skill artifacts.

Scenario this protects

If someone uploads a SKILL.md with normal-looking instructions near the top, then a very large whitespace gap, then important behavior after the gap, context-limited previews or model review may focus on the beginning and miss the later content.

ClawHub's static scan already reads the full text file. This PR teaches that deterministic layer to notice the evasive layout itself.

What changed

  • Adds suspicious.context_padding_truncation as a static scan reason code.
  • Flags extreme blank-line runs before later artifact content.
  • Flags extreme same-line whitespace runs before later artifact content.
  • Keeps the signal as static evidence for ClawScan, not a standalone hard block.
  • Adds regression tests for padded markdown, long whitespace padding, and ordinary sparse markdown that should remain clean.
  • Updates the moderation spec with the intent of the rule.

What maintainers should expect

A padded artifact can now produce a clear static finding that points reviewers and ClawScan to the evasion pattern. Ordinary markdown spacing should not be affected.

This does not change uploader bans, hide/unhide behavior, or final moderation policy by itself. It adds a signal the rest of the scan pipeline can use.

Validation

  • bunx vitest run convex/lib/moderationEngine.test.ts --reporter=verbose (88 passed)
  • bun run ci:static
  • bun run ci:unit (2,667 passed)
  • bun run ci:types-build
  • git diff --check upstream/main...HEAD

Behavior proof

Copied output from executing runStaticPublishScan against a redacted uploaded SKILL.md fixture with 2,500 blank lines before later content:

{
  "status": "suspicious",
  "reasonCodes": [
    "suspicious.context_padding_truncation"
  ],
  "findings": [
    {
      "code": "suspicious.context_padding_truncation",
      "severity": "warn",
      "file": "SKILL.md",
      "line": 2,
      "message": "Extreme context padding hides later artifact content from context-limited review.",
      "evidence": "2500 consecutive blank/whitespace-only lines before later artifact content; content resumes at line 2502."
    }
  ]
}

@vercel

vercel Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

@vyctorbrzezowski is attempting to deploy a commit to the Amantus Machina Team on Vercel.

A member of the Team first needs to authorize it.

@clawsweeper

clawsweeper Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 18, 2026, 4:15 PM ET / 20:15 UTC.

Summary
The branch adds a static moderation reason for extreme context padding, regression tests, GitHub-backed scan-status handling, and security spec notes.

Reproducibility: yes. for source-level behavior: current main has no context-padding detector, while the PR body includes copied static scan output and tests for padded artifacts. I did not run tests because this is a read-only review.

Review metrics: 3 noteworthy metrics.

  • Changed Surface: +430/-2 across 7 files. The PR spans scanner logic, GitHub-backed verification, tests, and security specs, so review must cover both runtime behavior and policy.
  • Mergeability: CONFLICTING / DIRTY. GitHub reports the live branch cannot merge against current main without conflict repair.
  • Runtime Route Drift: 0 Node wrapper files at PR head; 1 on current main. Current main moved GitHub archive fetch/expand into githubSkillSyncNode.ts, but the PR head predates that file.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦐 gold shrimp
Patch quality: 🧂 unranked krab
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Repair the branch against current main so the detector feeds prepareGitHubSkillScanRequestInternal and the Node wrapper remains intact.
  • Get maintainer sign-off or remove the padding-only clean-install exception.
  • [P1] Add redacted real behavior proof for the repaired GitHub-backed verification path.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body includes live output for the static detector, but it does not prove the GitHub-backed verification/installability path that the branch also changes; after repair, add redacted output or logs for that path as well. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Live GitHub mergeability is CONFLICTING/DIRTY, so the branch cannot land until it is repaired against current main.
  • [P1] The PR head applies static scan output directly as the GitHub scan status, bypassing current main's durable ClawScan request, file chunking, and worker verdict path.
  • [P1] The branch maps padding-only suspicious evidence to clean GitHub installability, which is a compatibility and security-boundary decision current specs do not yet endorse.
  • [P1] The PR head lacks the current githubSkillSyncNode.ts route, so a naive conflict repair could put GitHub archive fetch/expand back in the default action runtime.

Maintainer options:

  1. Repair Onto Current GitHub Scan Pipeline (recommended)
    Keep the new detector, but rebase it so GitHub verification still prepares and finalizes the durable ClawScan request through githubSkillSyncNode.
  2. Drop The Clean-Install Exception
    If maintainers do not want a new security-boundary exception, retain context-padding as static scan context and let the full GitHub ClawScan verdict alone control installability.
  3. Accept The Exception Deliberately
    Maintainers may choose to allow padding-only suspicious evidence to remain installable, but that needs an explicit spec and review decision after the runtime repair.

Next step before merge

  • [P1] The next action is maintainer-directed conflict repair plus a security-boundary decision on padding-only GitHub installability, not an autonomous cleanup close.

Security
Needs attention: The diff adds no dependency or supply-chain changes, but it changes the GitHub-backed moderation/installability security boundary and currently bypasses the full scan path.

Review findings

  • [P1] Route GitHub verification through the full scan request path — convex/githubSkillSync.ts:962-967
  • [P1] Keep GitHub verification in the Node wrapper — convex/githubSkillSync.ts:975-980
Review details

Best possible solution:

Rebase the detector onto current main so context-padding evidence feeds the existing full ClawScan pipeline through the Node wrapper, then have maintainers explicitly approve or reject any padding-only installability exception.

Do we have a high-confidence way to reproduce the issue?

Yes for source-level behavior: current main has no context-padding detector, while the PR body includes copied static scan output and tests for padded artifacts. I did not run tests because this is a read-only review.

Is this the best way to solve the issue?

No, not yet: the detector is plausible, but the PR must be repaired so it feeds current main's full GitHub ClawScan pipeline and preserves the Node wrapper. The padding-only clean-install exception also needs explicit maintainer acceptance or removal.

Full review comments:

  • [P1] Route GitHub verification through the full scan request path — convex/githubSkillSync.ts:962-967
    These lines make the static scan result the final GitHub verification result and immediately patch githubScanStatus. Current main now prepares a durable githubSkillScans/skillScanRequests pipeline and lets the full ClawScan verdict control installability, so the rebase needs to pass the new static finding into that path instead of replacing it.
    Confidence: 0.93
  • [P1] Keep GitHub verification in the Node wrapper — convex/githubSkillSync.ts:975-980
    At PR head, verifyGitHubSkillInternal is still exported from convex/githubSkillSync.ts and the branch has no convex/githubSkillSyncNode.ts. Current main moved sync and verification to the Node runtime because archive fetch/expand can exceed the default action runtime, so repair the branch with the wrapper and cron/scheduler references preserved.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.92

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 22d3cd133cb6.

Label changes

Label changes:

  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes live output for the static detector, but it does not prove the GitHub-backed verification/installability path that the branch also changes; after repair, add redacted output or logs for that path as well. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • remove proof: sufficient: Current real behavior proof status is insufficient, not sufficient.
  • remove status: ⏳ waiting on author: Current PR status label is status: 📣 needs proof.

Label justifications:

  • P2: This is a bounded moderation/security improvement with serious PR blockers but no evidence of an active production outage.
  • merge-risk: 🚨 compatibility: The PR changes GitHub-backed installability by allowing one suspicious static reason to map to a clean install status.
  • merge-risk: 🚨 security-boundary: The branch changes how deterministic scanner evidence affects the GitHub-backed trust gate and currently bypasses the full ClawScan verdict path.
  • merge-risk: 🚨 availability: The PR head predates the Node runtime wrapper required for GitHub archive fetch/expand, which can exceed the default action runtime.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦐 gold shrimp and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes live output for the static detector, but it does not prove the GitHub-backed verification/installability path that the branch also changes; after repair, add redacted output or logs for that path as well. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

Security concerns:

  • [high] Static scan bypasses the full GitHub ClawScan path — convex/githubSkillSync.ts:962
    The PR head applies the static scan result as the final GitHub verification result instead of preparing the current durable full-scan request, so static analysis alone could decide installability for GitHub-backed content.
    Confidence: 0.93
  • [medium] Padding-only suspicious evidence maps to clean installability — convex/githubSkillSync.ts:65
    githubScanStatusFromStaticScan returns clean when the only suspicious reason is context padding, which needs maintainer approval because it changes the trust gate for GitHub-backed installs.
    Confidence: 0.9

What I checked:

  • Live PR state: GitHub reports head 1245c11 as CONFLICTING/DIRTY with +430/-2 across 7 files. (1245c11a4d99)
  • Current main has no detector: A focused current-main search found no context-padding/truncation detector or reason code in Convex, specs, docs, or app source. (22d3cd133cb6)
  • Current main Node wrapper: Current main exports GitHub source sync and per-skill verification from githubSkillSyncNode.ts with "use node", so archive fetch/expand stays in the Node runtime. (convex/githubSkillSyncNode.ts:1, 22d3cd133cb6)
  • Current main cron routing: The current production cron schedules internal.githubSkillSyncNode.syncGitHubSkillSourcesInternal, not the default-runtime module. (convex/crons.ts:17, 22d3cd133cb6)
  • Node-runtime invariant: The GitHub-backed skills spec says source sync and per-skill verification must run in the Node runtime because archive fetch/expand can exceed default action memory. (specs/github-backed-skills.md:89, 22d3cd133cb6)
  • Full scan invariant: Current main requires GitHub-backed clean, suspicious, and malicious verdicts to persist on githubSkillScans, and says static findings alone must never promote or block the skill. (specs/github-backed-skills.md:149, 22d3cd133cb6)

Likely related people:

  • Patrick-Erichsen: Git blame and GitHub commit metadata show this author introduced the current GitHub sync Node wrapper, cron route, and spec/runtime invariants affected by the PR. (role: feature owner and recent area contributor; confidence: high; commits: 911515194911; files: convex/githubSkillSync.ts, convex/githubSkillSyncNode.ts, convex/crons.ts)
  • steipete: Git history shows repeated recent ClawScan and static-suspicious policy work in the same moderation/spec surface, including the Codex classification and advisory static-scan changes. (role: adjacent scanner policy contributor; confidence: medium; commits: 35aa372b24a3, c2c01300a757, 5d0264daa745; files: convex/lib/moderationEngine.ts, convex/lib/moderationReasonCodes.ts, specs/security-moderation.md)
  • vincentkoc: Git history shows many recent deterministic security detector commits in the moderation engine and reason-code area that this PR extends. (role: adjacent static-scan contributor; confidence: medium; commits: b4cfe3365954, 46354c9967f2, fc74a2f6cd87; files: convex/lib/moderationEngine.ts, convex/lib/moderationReasonCodes.ts, specs/security-moderation.md)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jun 4, 2026
@vyctorbrzezowski vyctorbrzezowski marked this pull request as ready for review June 4, 2026 04:47
@vyctorbrzezowski vyctorbrzezowski requested review from a team and Patrick-Erichsen as code owners June 4, 2026 04:47
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. label Jun 4, 2026
@vyctorbrzezowski

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Added redacted runStaticPublishScan behavior proof to the PR body. Validation: focused moderation tests, ci:static, ci:unit, and ci:types-build all pass.

@clawsweeper

clawsweeper Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 4, 2026
@vyctorbrzezowski

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jun 7, 2026
@vyctorbrzezowski vyctorbrzezowski force-pushed the brzezowski/context-padding-static-scan branch from c08969e to 63fe7fd Compare June 8, 2026 06:53
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. proof: sufficient Contributor real behavior proof is sufficient. labels Jun 8, 2026
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 11, 2026
@vyctorbrzezowski vyctorbrzezowski force-pushed the brzezowski/context-padding-static-scan branch from 463c299 to 1245c11 Compare June 15, 2026 15:39
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant