Harden Portable Skill Safety Boundaries - #9
Conversation
|
Reviewed current status. Hosted checks pass, but this draft conflicts with master. Its explicit package release block remains active. Do not rebase, merge, tag, or publish until that block is lifted. A different human reviewer is also required before merge. |
Signed-off-by: kriptoburak <kriptoburak@users.noreply.github.com>
38e15b3 to
7ea93f5
Compare
📝 WalkthroughWalkthroughThe update adds explicit approval, privacy, retention, credential, and destructive-action guidance across Xquik endpoint references and task guides. It also scopes DM history access to an explicitly selected account and adds an MIT license declaration. ChangesSafety and approval guidance
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Rebuilt on current |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ea93f5c27
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: kriptoburak <kriptoburak@users.noreply.github.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
All reported issues were addressed
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Fix all with cubic | Re-trigger cubic
Signed-off-by: kriptoburak <kriptoburak@users.noreply.github.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Signed-off-by: kriptoburak <kriptoburak@users.noreply.github.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
4 issues found across 15 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="skills/x-twitter-scraper/references/api-endpoints-tweet-style-cache.md">
<violation number="1" location="skills/x-twitter-scraper/references/api-endpoints-tweet-style-cache.md:56">
P2: The list gate refers to an account scope that is not an input to `GET /styles`; the endpoint returns all cached style profiles, up to 200. Approval should explicitly cover the complete returned list, otherwise “account scope” leaves the amount of cached-profile metadata being disclosed ambiguous.</violation>
</file>
<file name="skills/x-twitter-scraper/references/api-endpoints-extractions.md">
<violation number="1" location="skills/x-twitter-scraper/references/api-endpoints-extractions.md:95">
P2: Listing extraction history can proceed without approval of the exact account, purpose, downstream recipients, or retention plan because this endpoint-local gate scopes approval only to filters and pagination. Include those private-read dimensions so “that exact read” cannot be interpreted as approval based only on query parameters.</violation>
<violation number="2" location="skills/x-twitter-scraper/references/api-endpoints-extractions.md:106">
P2: Retrieving up to 1,000 extraction results can proceed without approval of the exact account, purpose, downstream recipients, or retention plan because this gate names only the job and page. Add the omitted scope so approval covers how this private result set will be used and handled.</violation>
<violation number="3" location="skills/x-twitter-scraper/references/api-endpoints-extractions.md:117">
P2: The export approval requirements differ between the two extraction reference docs for the same endpoint. `api-endpoints-extractions.md` says to show "enrichment risk" while `extractions.md` says to show "enrichment columns"; one is a risk warning, the other is concrete column disclosure. Similarly, "recipient" vs "audience" and "full scope" vs "full fixed dataset" describe overlapping but distinct scoping concepts. A developer following one doc could miss requirements present in the other, leading to inconsistent approval flows. Align both files on the same set of disclosure and approval items for the export endpoint.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
furkanerday
left a comment
There was a problem hiding this comment.
Four actionable safety-documentation findings remain unresolved on the current head.
skills/x-twitter-scraper/references/api-endpoints-extractions.md:96must bind extraction-history approval to the exact account, purpose, filters/page bound, downstream recipients, and retention plan.skills/x-twitter-scraper/references/api-endpoints-extractions.md:107must apply the same complete scope to result retrieval.skills/x-twitter-scraper/references/api-endpoints-extractions.md:117andskills/x-twitter-scraper/references/extractions.md:229must use one consistent disclosure set for the same export endpoint (fixed dataset, enrichment columns, audience/recipients, storage, and retention).skills/x-twitter-scraper/references/api-endpoints-tweet-style-cache.md:56must state thatGET /stylesreturns the complete cached-profile list (up to 200); it has no account-scope input.
Resolve the live threads, re-run the required checks, and re-request review.
Signed-off-by: kriptoburak <kriptoburak@users.noreply.github.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/x-twitter-scraper/references/api-endpoints-api-keys.md`:
- Around line 5-10: Make the API-key lifecycle documentation non-actionable by
removing the callable request/response details for POST /api-keys, GET
/api-keys, revocation, and the fullKey response, or placing them behind an
explicit block directing users not to call these routes from this Skill and to
use the Xquik dashboard account page instead. Keep the existing Agent Boundary
consistent with the revised content.
In `@skills/x-twitter-scraper/references/api-endpoints-draws.md`:
- Around line 21-23: Add lawful purpose, export audience, and retention plan as
explicit fields in the approval record described by the create gate, alongside
participant-data handling. Ensure the draw-creation approval requires these
values before persisting any participant data.
- Around line 72-74: Update the private-read approval guidance in the draws
endpoint documentation to bind both GET /draws and GET /draws/{id} approvals to
the exact account, while retaining the required page or draw-ID scope. For
draw-detail approvals, also record the scope of the returned data, and apply the
same changes to the corresponding section noted for the additional occurrence.
In `@skills/x-twitter-scraper/references/api-endpoints-extractions.md`:
- Around line 3-12: Make the bounded extraction contract consistent in
skills/x-twitter-scraper/references/api-endpoints-extractions.md lines 3-12 by
requiring a finite resultsLimit or documenting and enforcing a finite default.
Update skills/x-twitter-scraper/references/extractions.md lines 15-17 to state
the same exact-bound requirement and ensure it matches the endpoint behavior.
- Around line 119-126: Replace the full-dataset pre-approval disclosure in
skills/x-twitter-scraper/references/api-endpoints-extractions.md lines 119-126
with only a bounded preview, schema, row count, and field list; materialize or
transmit the complete dataset only after explicit approval. Apply the same
change to skills/x-twitter-scraper/references/extractions.md lines 231-238,
preserving the existing approval and export safeguards.
In `@skills/x-twitter-scraper/references/api-endpoints-monitors.md`:
- Around line 99-101: Update the keyword monitor approval guidance in the
monitor endpoints section to include operation-specific previews: show proposed
keywords, event types, delivery changes, and active-state transitions for
create, update, enable, and disable operations; for deletion, show the exact
target and associated data that will be permanently lost. Align these
disclosures with the existing primary monitor endpoint requirements.
- Line 15: Label every affected fenced block with an appropriate language
identifier to satisfy markdownlint MD040: use an HTTP-oriented identifier for
POST /monitors at
skills/x-twitter-scraper/references/api-endpoints-monitors.md:15-15, GET
/monitors/{id} at :59-59, PATCH /monitors/{id} at :68-68, POST /webhooks at
skills/x-twitter-scraper/references/api-endpoints-webhooks.md:15-15, PATCH
/webhooks/{id} at :46-46, and POST /webhooks/{id}/test at :67-67; use text
instead where HTTP is not appropriate.
In `@skills/x-twitter-scraper/references/api-endpoints-x-accounts.md`:
- Around line 28-30: Make private-read approval operation-specific and
consistent across all three references: in
skills/x-twitter-scraper/references/api-endpoints-x-accounts.md lines 28-30,
require explicit approval for the complete connected-account list; in
task-guides/send-dms.md lines 62-65, gate GET /x/accounts before sender
selection or require a user-supplied account; and in
skills/x-twitter-scraper/references/workflows.md line 191, mark DM history
private, require exact-account approval, and block ambiguous account selection.
In `@skills/x-twitter-scraper/references/api-endpoints-x-api.md`:
- Around line 192-194: Add an http or text language tag to the opening endpoint
code fences in skills/x-twitter-scraper/references/api-endpoints-x-api.md lines
192-194 and skills/x-twitter-scraper/references/api-endpoints-x-write.md lines
17-19.
- Around line 186-188: Update the documentation for GET /x/bookmarks/folders to
state that it reads the authenticated caller’s connected account because it has
no account parameter. Document how the active account is selected when multiple
accounts are connected, and require confirmation for that exact account
immediately before performing the private read.
In `@skills/x-twitter-scraper/references/python-examples.md`:
- Around line 15-20: Update the xquik_fetch retry behavior described in the
examples so POST, PATCH, and DELETE requests are not retried automatically after
429 or 5xx responses. Restrict automatic retries to idempotent methods, or
require both an idempotency key and explicit opt-in before retrying write
operations.
In `@skills/x-twitter-scraper/references/workflows.md`:
- Around line 16-21: Update the extraction workflow around the POST /extractions
and export endpoint calls to enforce the documented approval requirement: add
explicit approval checkpoints immediately before creating the metered job and
before exporting results, or clearly mark both calls as non-executable
placeholders. Preserve the existing warning and workflow context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f83e6674-32b9-4e40-aa5e-40a7b0ea03b8
📒 Files selected for processing (18)
skills/x-twitter-scraper/references/api-endpoints-api-keys.mdskills/x-twitter-scraper/references/api-endpoints-drafts.mdskills/x-twitter-scraper/references/api-endpoints-draws.mdskills/x-twitter-scraper/references/api-endpoints-extractions.mdskills/x-twitter-scraper/references/api-endpoints-monitors.mdskills/x-twitter-scraper/references/api-endpoints-support.mdskills/x-twitter-scraper/references/api-endpoints-tweet-style-cache.mdskills/x-twitter-scraper/references/api-endpoints-webhooks.mdskills/x-twitter-scraper/references/api-endpoints-x-accounts.mdskills/x-twitter-scraper/references/api-endpoints-x-api.mdskills/x-twitter-scraper/references/api-endpoints-x-media.mdskills/x-twitter-scraper/references/api-endpoints-x-write.mdskills/x-twitter-scraper/references/extractions.mdskills/x-twitter-scraper/references/mcp-tools.mdskills/x-twitter-scraper/references/python-examples.mdskills/x-twitter-scraper/references/workflows.mdskills/xquik-social-research/SKILL.mdtask-guides/send-dms.md
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Signed-off-by: kriptoburak <kriptoburak@users.noreply.github.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
furkanerday
left a comment
There was a problem hiding this comment.
Two blockers remain on current head 0eddb3a. First, skills/x-twitter-scraper/references/python-examples.md:72-85 sends both the estimate and create requests without the finite positive resultsLimit that the changed policy makes mandatory, and it reaches POST /extractions after only allowed=true with no exact-job approval checkpoint. This can start an unbounded metered bulk-data job outside the user's approved scope; mirror the fail-closed bound and approval pattern already present in workflows.md:94-122 and add parity coverage. Second, human-authored merge commit 839cdea (Burak Bayır burakbayir1@gmail.com) has no author-matching Signed-off-by trailer. Please amend or recreate that commit with the contributor's own sign-off and re-push it. The current required checks are green and prior threads are resolved, but neither blocker can be approved; review the new head from scratch after correction.
Summary
Audit
This PR was rebuilt on current
master. It excludes the stale 2.5.5 release bump and scan metadata. It does not publish, tag, or release anything.Verification
npm cinpm testnpm run check:reproduciblenpm run check-versionsAll checks pass locally.
Note
Harden safety boundaries across x-developer skill API reference documents
accountquery parameter requirement and exact-account approval enforcement to the DM history endpoint across api-endpoints-x-api.md, mcp-tools.md, workflows.md, and send-dms.md.license: MITto xquik-social-research/SKILL.md front matter.Changes since #9 opened
Export Extractionendpoint to prohibit row/field projection, require setting the smallest approvedresultsLimitat job creation, mandate showing exact account, job ID, purpose, format, full fixed dataset, enrichment columns, enrichment risk, all downstream recipients, storage location, and retention period before approval, enforce blocking exports exceeding approved purpose, and require deletion when approved purpose ends [e4cf119]List Extractions,Get Extraction, andList Stylesendpoints to require showing exact account, purpose, scope, downstream recipients, and retention plan before approval for private read operations [e4cf119]POST,PATCH, andDELETErequests [0eddb3a]resultsLimitparameter for all extraction operations [0eddb3a]Macroscope summarized 839cdea.
Summary by CodeRabbit