Repository navigation
feat(plugins): add NkAntony777/scansci-pdf academic paper retrieval plugin - #64
NkAntony777 wants to merge 13 commits into
Conversation
…lugin Wraps the upstream scansci-pdf engine (Rimagination/scansci-pdf, Apache-2.0, v1.17.0) as two Skills (download/search/troubleshoot + large-list triage) and a stdio MCP server (scansci-pdf run). Engine installed from PyPI, not vendored; credentials and config stay local.
- add .minimax-plugin/plugin.json (MiniMax V1 manifest) required by marketplace validation - rename mcp.json -> scansci-pdf.mcp.json and align with *.mcp.json schemaVersion format - add icon.png referenced by the manifest
- re-encode icon.png to a clean 512x512 RGBA PNG (drops the non-standard caBX chunk carried from the source image, matching the standard chunk profile of icons already accepted by the marketplace) - bump package version after content change per submission guide
…17.2 The description contained 'DOIs): classify' - a colon+space inside an unquoted YAML scalar breaks frontmatter parsing (Marketplace V1 check SKILL_FRONTMATTER_INVALID). Reworded to a comma construction; verified both SKILL.md frontmatters parse with a YAML parser.
hetaoBackend
left a comment
hetaoBackend
left a comment
There was a problem hiding this comment.
Request changes for exact current head b523ef43ec9fef79c83dc03efbdba07b689290d6.
Blocking supply-chain/security/host-contract issues:
- The public
oa_urlqueue field is fetched with redirects and no loopback/private/link-local/metadata/DNS-rebinding policy. A controlled loopback PDF URL was accepted and requested. Enforce per-hop public HTTPS/IP validation at the actual connection boundary and add redirect/rebinding regressions. - The MCP-exposed Tor installer downloads a tar and uses unchecked
tarfile.extractall(), then marks discovered binaries executable. A../member was reproduced writing outside the target directory. Reject absolute/traversal/link members and verify a pinned signature/hash before extraction/execution. - Plugin manifests say 1.17.2, but that upstream/PyPI version does not exist; README references 1.17.0, installation is unpinned, and MCP executes bare
scansci-pdffrom PATH. Pin an existing audited package plus integrity/provenance and bind the launched executable/version. springer_api_keyis omitted from masking and can be returned byscansci_pdf_config; config/cookie state is written without restrictive mode (reproduced as 0644 under umask 022). Mask all credentials and use 0600/symlink-safe persistence.- Core Skill workflows reference
scripts/sort_finalize_writeback.pyandscripts/sort_mdpi_res_batch.py, but those scripts are not packaged. The nonstandardscansci-pdf.mcp.jsonis also not validated by the repository’s rootmcp.jsoncontract. Ship and test the actual package/host layout. - Cache/output/destructive paths lack workspace containment, symlink policy and confirmation; paper content is returned into model context without an explicit prompt-injection trust boundary.
Exact-head CI/CodeQL are action_required; the only visible check is [code]smith = SKIPPED, which is not evidence.
|
@hetaoBackend — updated PR #64 to commit Thanks for the detailed review. This revision vendors and patches the engine directly, based The actual connection layer now validates DNS answers and uses numeric destination addresses, Local Windows evidence: 429 engine tests passed, 3 skipped; actual stdio MCP integration and REVIEW-NOTES.md contains the details and limitations: remote browser daemons/private-intranet Additional disclosure: the repository-wide Please re-review the updated head when convenient. |
hetaoBackend
left a comment
hetaoBackend
left a comment
There was a problem hiding this comment.
Request changes for exact current head f3d8b1c9529d3221b4b423d995897263f1da04b1.
The unified public-HTTPS transport and source-hash checks improve the previous SSRF and vendoring findings, but this exact head still exposes multiple merge-blocking security surfaces:
- RemoteAssist is an unauthenticated LAN control surface.
vendor/scansci-pdf/src/scansci_pdf/remote_assist.py:229-237bindsHTTPServer(("0.0.0.0", port), ...); focused probes received unauthenticated 200 responses from/api/statusand/api/done. Bind to loopback by default; if LAN access is required, add explicit owner confirmation, short-lived authentication and CSRF protection. Escape dynamicpublisher/browser_urlvalues before inserting them into the HTML template. - The optional web UI also defaults to unauthenticated
0.0.0.0.src/scansci_pdf/main.py:64-78defaultsweb --hostto0.0.0.0;src/scansci_pdf/web.py:97-190exposes unauthenticated/api/download,/api/search, and/api/status. Bind loopback by default or remove this entrypoint from the hosted package; add authentication, CSRF protection, and complete workspace/path containment for downloads. - Proxy credentials can be disclosed in diagnostics and recommendations.
vendor/.../sources/scoring.py:153-165,201-213places rawSCANSCI_PDF_PROXY/config proxy URLs into report fields and error recommendations. This includesuser:password@hostforms and bypasses the otherwise improved masking path. Redact structurally at every MCP/CLI/health/recommendation/error boundary, including empty usernames, IPv6 and encoded credentials; never return raw proxy URLs. - Browser login persists more credential state than necessary.
vendor/.../browser_login.py:171-187falls back from publisher cookies to the entire cookie jar;browser_cookies.py:108-150persists all cookies and all discovered localStorage origins. This can retain unrelated IdP/SSO sessions and other domains. Restrict custom login destinations to an allowlist, save only the target-domain minimum cookie fields, disable localStorage persistence by default, and require explicit confirmation for any broader capture. - The secure launcher is not self-contained in a clean environment.
vendor/scansci-pdf/run_secure.py:52-83importspackagingbutvendor/scansci-pdf/pyproject.tomldoes not declare it as a direct dependency; isolatedrun_secure.py --verifyfails withModuleNotFoundError: packaging. Add it to the package dependency and lock/bootstrap contract, then run the launcher and actual stdio MCP in a clean environment. Version-only checks are not independent provenance for the executing distribution. - Browser executable provenance remains uncontrolled. The browser backend accepts PATH-discovered or arbitrary configured executables without a release signature/hash and does not apply the same symlink/provenance gate consistently. Restrict to verified allowlisted binaries and document the install/update provenance.
Repository validation passed and the vendored source hash manifest matched, but Python tests, actual stdio MCP, browser/Tor integration, and launcher verification were not independently green in this review environment. [code]smith = SKIPPED was not used as evidence. Do not approve or merge until the network surfaces, credential persistence/redaction, launcher dependency contract, and exact-head runtime evidence are fixed.
…enance Addresses the six merge-blocking findings on the previous head. RemoteAssist bound 0.0.0.0 and answered unauthenticated /api/status and /api/done, so anyone on the LAN could drive the verification flow. It now binds 127.0.0.1 unless the caller passes allow_lan=True *and* the config sets remote_assist_lan, and every request carries a per-start bearer token compared with hmac.compare_digest. The loopback bind is authenticated too: every process on the host can reach 127.0.0.1, and one code path means auth cannot be skipped by accident. POST /api/done additionally requires X-Requested-With, which a cross-site form post cannot set. The page's publisher and browser_url are html.escape'd before interpolation. The optional web UI defaulted --host to 0.0.0.0 behind unauthenticated /api/download, /api/search and /api/status. The default is now 127.0.0.1 and the endpoints require a bearer token plus the same CSRF header. /api/download resolves and containment-checks the returned path against the configured output directory and refuses symlinks, instead of serving any path the download layer returned. This also fixes a pre-existing defect: _PAGE_TEMPLATE.format() raised KeyError on the inline CSS braces, so the assist page returned 500 on every request. Slots are filled explicitly instead. Raw proxy URLs reached report fields, recommendations and error strings. redact_proxy_url delegates to the existing config.mask_config_value rather than adding another implementation, and covers the empty-username form that helper missed; mask_config_value itself now masks that form too. No raw proxy URL is returned on any boundary, including the ProxyError path. Browser login no longer falls back to the entire cookie jar, and cookies and localStorage are persisted scoped to the target domain; localStorage is opt-in. Custom login destinations are allowlisted, and the publisher-cookie match is dot-anchored so evil-sciencedirect.com is no longer accepted. run_secure.py imported packaging without declaring it, so an isolated --verify failed with ModuleNotFoundError. It is now a declared dependency, and a missing one produces an actionable install message. --verify also states what it actually proves: local hashes and installed dependency versions, not upstream provenance. Browser executables were accepted from PATH or arbitrary config. A new provenance gate requires an allowlisted system-browser root or the engine cache, reuses the existing no_symlinks gate, and honours a pinned digest; SCANSCI_PDF_TRUST_BROWSER_PATH is the explicit opt-out. Provenance here is "the file the owner approved", not a vendor code-signature check. 493 tests pass. The 2 remaining failures need the optional camoufox and cloakbrowser backends and fail identically before these changes. No live SSO round-trip was exercised: cookie scoping is covered by unit tests.
…t survived Follow-up to the previous head, addressing what a second review could still reach. Proxy credentials still escaped through paths other than network_diagnose. The failure-tips builder in sources/__init__.py interpolated raw proxy URLs into tips returned to the model and the CLI, config-cmd echoed the value it was given and printed network_proxy and browser_static_proxy directly, and a credential with no scheme (user:pass@host) was passed through unredacted. mask_config_value is now the single implementation and covers the schemeless form; the duplicate regex in redact_proxy_url is gone. One test pinned the schemeless pass-through; that expectation encoded the leak and was inverted. Cookie scoping stayed wider than the target domain: scoped_cookies kept every known publisher's cookie, so one login left a live session behind for all of them. It is now target-domain only. WebVPN/EZProxy are unaffected because they proxy the publisher under the proxy hostname, which is the target. _is_publisher_cookie is dot-anchored so evil-sciencedirect.com cannot match. browser_persist_localstorage joins the confirmed:true set in strict mode, so the model can no longer enable it without owner confirmation. Browser provenance had three bypasses. channel="chrome" — the default backend path — launched whatever Playwright resolved, ungated; the browser is now resolved through the gate and passed as executable_path, and the bundled Chromium retry is gated too. CLOAKBROWSER_BINARY_PATH was accepted without validation. And SCANSCI_PDF_TRUST_BROWSER_PATH returned before the digest comparison, so the escape hatch also disabled hashing; it now relaxes only the allowlist, never a pinned digest. Cache-root binaries require a pinned digest because the cache is writable at runtime. Per-user installs are reachable but also require a digest, so a per-user Chrome is a configuration step rather than a dead end. Token exposure was wider than the authenticated routes. The web page rendered the bearer token into an inline script while loading Tailwind and Alpine from two CDNs, any of which could read it and call the local API. The token is no longer rendered at all: the ?t= link hands it over once as an HttpOnly, SameSite=strict cookie, and page JavaScript authenticates via that cookie. A CSP now constrains script/style/connect sources on every response, and /docs, /redoc and /openapi.json are disabled on an authenticated surface. uvicorn's access log records the full request target, so it no longer runs at all for the web command. RemoteAssist keeps its inline token but serves a CSP that permits no external script, stops logging the authenticated URL, and refuses an expired token even when the constant-time comparison would match. 517 tests pass. The 4 remaining failures need the optional camoufox, cloakbrowser and pymupdf4llm packages and fail identically before these changes. Manifest regenerated and verified in both directions (167 entries, no mismatches, no tracked source missing). No live SSO round-trip was exercised; cookie scoping is covered by unit tests.
…arty Program Files was treated as a user-writable root, so the stock Chrome and Edge installs were rejected without a pinned digest, and a rejected bundled Chromium aborted a browser that had already passed the gate. The web UI no longer loads third-party scripts. Its own code is nonce-bound, and setup --show masks proxy passwords. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@hetaoBackend — updated PR #64 to commit
Local evidence: the provenance, token-exposure, and proxy-redaction tests passed (60 passed, 1 skipped because this environment cannot create symlinks). I am not counting approval-pending GitHub jobs as passes. Please re-review the updated head when convenient. |
hetaoBackend
left a comment
hetaoBackend
left a comment
There was a problem hiding this comment.
Request changes for exact current head 9e83a1bccc8a48e7b0f58a0edee746e207fb3968.
The new head closes important portions of the prior review: RemoteAssist and the optional Web UI default to loopback with auth/CSRF, proxy diagnostics in scoring.py are redacted, packaging is declared and locked, and source-hash verification is stronger. The following exact-head blockers remain:
- Browser credential persistence still has bypass paths. Although
browser_login.pyhas scoped/minimal helpers, restore accepts all cookies/localStorage frombrowser_state.jsonwithout reapplying domain scope or the localStorage opt-in. Other login flows directly persist fullcontext.cookies(), includingauth.py,instsci.py, and_publisher_strategies_core.pythroughsession_store.py. This can retain unrelated IdP/SSO and other-publisher sessions. Route every persistence/restore path through target-domain filtering and minimum fields; default localStorage persistence off and enforce the same policy on restore. - Camoufox launch paths are not provenance-gated.
browser_backend.py:588-691callsNewBrowser(...)directly; strict mode only checks the installation directory exists (:736-741,:786-789) and does not verify the actual executable with the Patchright-style symlink/signature/hash gate. An altered binary inside the directory can therefore execute despite the new provenance policy. Gate both ordinary and persistent Camoufox launches on the actual executable and verified provenance. - Exact-head runtime evidence is absent. GitHub reports zero check-runs/statuses for this head (
gh pr checksreports no checks), so the workflow file and producer's local “429 passed” claim do not establish the Python suite, clean launcher, stdio MCP, browser, Tor, or Windows/Ubuntu contract. Run and retain the exact-head workflow results before approval. - The explicit non-loopback streamable HTTP mode remains unauthenticated.
main.py:50-54launchesmcp_app.streamable_http_app()directly when--mode streamable_http --host 0.0.0.0is supplied, bypassing the Web UI's auth middleware. Either reject non-loopback HTTP without an authenticated transport or enforce auth at this entrypoint. - Proxy redaction is not a single egress boundary. Raw proxy values still reach at least
auth.pylogging/launch arguments and other source failure/result paths outsidescoring.py. Ensure logs, process arguments, MCP results and exception/source-failure objects cannot contain credential-bearing proxy URLs.
node validation/source-hash checks are positive, but they do not close these runtime and provenance gaps. [code]smith = SKIPPED was not used as evidence. Do not merge this head.
… 3.11 floor The browser and base locks pinned networkx==3.7 and numpy==2.5.3, which exclude Python 3.11 (networkx 3.7 via file-level Requires-Python, numpy 2.5.x by shipping only cp312+ wheels). requires-python is >=3.11 and the engine CI matrix includes 3.11, so the first real CI run failed at dependency install. Repin to the highest 3.11-compatible versions (networkx 3.6.1, numpy 2.4.6) with full PyPI hashes in both locks and regenerate SOURCE-HASHES.json. Verified: hash-locked installs on Python 3.11 and 3.13; full engine suite 767 passed / 5 skipped on both.
The first Linux CI run exposed four test-harness assumptions that held only on a wide Windows console: - Typer help assertions matched raw stdout; at CI's 80-column default the Rich output wraps and styles those substrings away. Normalize ANSI and whitespace before asserting. - Two stdio lifecycle tests closed stdin by hand before communicate(); POSIX Popen.communicate() then flushes the closed stdin and raises. communicate(input=...) writes, flushes and closes stdin itself. - The taskkill tree-kill test relied on the host being Windows to select the branch; patch os.name like the posix test does. - The real-Chromium guard ignored SCANSCI_AUDIT_BROWSER and hardcoded the Windows Chrome path, so the ubuntu leg skipped the round trip. Read the env var (Windows default unchanged) and point the ubuntu matrix at /usr/bin/google-chrome. Full suite 767 passed / 5 skipped at COLUMNS=80 with the real Chrome round trip enabled.
…e real Chrome binary Two leftovers from the first Linux run: - The staged stdio handshake test closed stdin immediately after the write, racing the server's EOF shutdown against the tools/list answer on fast hosts: initialize was answered, tools was dropped. Read the responses with stdin still open (a real client waits before closing), then close to confirm the clean exit. - The ubuntu matrix pointed at /usr/bin/google-chrome, which is a symlink to /opt/google/chrome/chrome; the provenance gate refuses symlinks by design, so the real-browser round trip failed with SecurityError. Point at the real binary.
On POSIX, Popen.communicate() flushes self.stdin unconditionally, so calling it after a manual stdin.close() raises 'flush of closed file' even with input=None — exactly what the handshake test hit after waiting for the tools response. Drain both pipes with reader threads and await the child with wait() instead; the finally path gets the same treatment (a second communicate() after the first failed mid-state raised AttributeError: no attribute '_fileobj2output').
|
Reply to the third-round review (head
Local verification at this head: 767 passed / 5 skipped on Windows Python 3.11 and 3.13 with Not claimed: real Camoufox/Firefox launch evidence, Linux desktop testing beyond CI, or physical power-loss durability. |
2 participants
Add scansci-pdf — academic paper retrieval plugin
Problem
MiniMax Code users who do literature work need paper retrieval: download by DOI/arXiv, search
and export citations, and bulk-process reading lists of hundreds to thousands of entries.
Doing this by hand (per-site browsing, manual OA checks, re-downloading failures) is slow and
error-prone. scansci-pdf (upstream, Apache-2.0,
v1.17.0) solves this with a 20+ source hedged cascade, lane-scheduled batch downloading, and
user-authorized institutional routes. This PR packages it as an Agent Plugin: two Skills plus a
stdio MCP server.
Example prompt
Expected result: the agent resolves the DOI, downloads the PDF (reporting which source produced
it, typically an open-access direct link within seconds), saves it to the working directory,
and returns a verified BibTeX record.
Dependencies and platforms
scansci-pdfexecutable onPATH(pip install scansci-pdforuv tool install scansci-pdf; the PyPI wheel ships a prebuilt Cython core, no compilerneeded). Optional headless-browser extra:
scansci-pdf[camoufox].scansci-pdf runover stdio; CLI fallback works without MCP).contains only markdown and JSON (no binaries, no credentials, no installers).
Network and data behavior
Scholar, Europe PMC/PMC, arXiv, DOAJ, OpenAIRE, publisher sites/CDNs (Elsevier, Springer,
MDPI), Sci-Hub mirrors, LibGen, and the user's own institutional WebVPN/CARSI endpoints.
config_needed/ask_user_emailwhen missing — never silently skipped).WebVPN/CARSI browser login). Config, credentials, and cookies stay local in
~/.scansci-pdf/.legal_onlystrategyrestricts to lawful sources.
Ownership and maintenance
Upstream engine by Rimagination (Apache-2.0, Copyright 2024-2026 scansci-pdf contributors —
LICENSE in the plugin dir is upstream's verbatim). This package redistributes the two Skill
documents and MCP wiring from upstream v1.17.0 and tracks upstream releases. Submitted and
maintained here by @NkAntony777.
Test evidence
npm run check:OK plugin NkAntony777/scansci-pdf(hosted-plugin validator: manifest,Skills, MCP transport, required docs, placeholders, path safety).
scansci-pdf checkandscansci-pdf run --mode stdioverified working on Windows 11 withPython 3.13 (
uv tool install scansci-pdf), including a live MCP session in another agenthost (ZCode) using the same skills + stdio server wiring.
tests/plugins/octopus-meme-maker/smoke.test.mjsfail onmainboth withand without this change (verified via stash) — pre-existing, unrelated to this PR.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.