[spark-compete] fix: clarification TTL bypass — reject expired entries without fallback - #837
Open
binance1230 wants to merge 1 commit into
Open
Conversation
…thout fallback shouldUsePendingClarificationForMessage previously required isPendingClarificationFollowup(text) to return true even when the clarification had not expired. This caused a TTL bypass where expired clarifications could still be used if the followup text matched. Now the function simply rejects expired entries and accepts non-expired ones, removing the incorrect coupling between TTL and followup detection.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
{ "schema": "spark-compete-hotfix-v1", "event": "spark-compete-first-event", "submission_mode": "public_repo_pr", "submission_target_url": "https://github.com/vibeforge1111/spark-telegram-bot/pull/837", "team": { "name": "king", "members": [ "@king1005678", "@AtwoodJonathan1", "@JaredAddison12" ], "llm_device_holder": "@king1005678", "device_holder_github": "binance1230", "github_accounts": [ "binance1230" ] }, "target_repo": { "id": "vibeforge1111/spark-telegram-bot", "source": "https://github.com/vibeforge1111/spark-telegram-bot", "owner_surface": "telegram-bot" }, "issue": { "type": "bug", "severity": "high", "title": "Clarification TTL bypass \u2014 expired entries still usable via followup detection", "actual_behavior": "shouldUsePendingClarificationForMessage requires isPendingClarificationFollowup(text) to be true even when the clarification has not expired. This creates a TTL bypass: expired clarifications that match the followup pattern can slip through because the coupling between TTL and followup detection is incorrect.", "expected_behavior": "Expired clarification entries should be rejected outright. Non-expired entries should be accepted without requiring followup text detection, as the TTL check is the sole gate for expiry.", "repro_steps": [ "1. Trigger a pending clarification for a user.", "2. Wait for the TTL to expire (30 minutes).", "3. Send a followup message that matches isPendingClarificationFollowup.", "4. Observe that the expired clarification is incorrectly used instead of being rejected." ], "affected_workflow": "Telegram bot clarification handling" }, "evidence": { "safe_links_only": true, "before_after_proof": "Before: expired entries bypass TTL if followup text matches. After: expired entries always rejected, non-expired entries always accepted.", "links": [ "https://github.com/vibeforge1111/spark-telegram-bot/pull/837" ], "forbidden": [ "No raw logs, private conversations, secrets, tokens, screenshots of private chats, or archive downloads." ] }, "proposed_fix": { "approach": "Separate the TTL expiry check from the followup text detection in shouldUsePendingClarificationForMessage. Return false immediately for expired entries, return true for non-expired entries without requiring isPendingClarificationFollowup.", "files_expected": [ "src/telegramPendingBuildEvidence.ts" ], "tests_or_smoke": "smoke_test: function returns false for expired entries regardless of text, returns true for non-expired entries" }, "pr": { "url": "https://github.com/vibeforge1111/spark-telegram-bot/pull/837", "branch": "fix/ttl-bypass-pending-clarification", "title_prefix": "[spark-compete]", "author_github": "binance1230", "body_must_include": [ "packet", "team", "pr_author", "repo", "actual_behavior", "expected_behavior", "repro_steps", "before_after_proof", "tests_or_smoke", "duplicate_notes", "risk_notes", "review_claim" ], "replaces_url": "https://github.com/vibeforge1111/spark-telegram-bot/pull/87" }, "review_claim": { "impact_claim": "high", "evidence_types": [ "smoke_test", "redacted_conversation_excerpt" ], "duplicate_notes": "Replaces #87 (which contained stacked fixes). This PR addresses only the TTL bypass root cause.", "risk_notes": "Low risk: changes only the return logic of shouldUsePendingClarificationForMessage. The function is used by pendingBuildClarificationForMessage which already handles the null case.", "review_state_requested": "pr_review" } }