Skip to content

[Review] open-code-review 扫描结果:201 条,其中 9 条已修、6 条已核实的严重项待认领 #1775

Closed

Description

@Ikalus1988

怎么来的

用 alibaba/open-code-review(ocr 1.12.4,npm CLI,指向本地 OpenAI 兼容端点)跑了三次 ocr scan:

范围 文件 条数 用时 tokens
misakanet-setup.mjs 1 9 2m57s 155k
核心脚本(lesson_gate / intake_auto_review / sync_lesson_count / check_provenance / checkpoint_reminder / install_misakanet_agent / copy-hook) 7 32 4m27s 1.04M
misakanet/ 包 48 160 18m58s 2.38M

每条都是"机器发现,人工核实":下面标 ✅ 的是我逐条读过代码确认的(其中 9 条已在 #1773 / #1774 修掉),其余标注为「未核实」,需要接手者自己判断——本轮已经见过一个反例:hasTopLevelKey 那条是误报(TOML 规范里 [table] 之后的裸键属于该表),所以未核实的条目不要当成事实。

已核实的严重项(本轮未修,建议优先)

  • misakanet/freshness.py — freshness_boosts 列表被解析成字典(critical,实测确认):_extract_frontmatter() 对
    freshness_boosts:
      - type: was_used
      - type: helpful_vote
    
    返回 {'- type': 'helpful_vote'}。下游 compute_freshness 里 isinstance(boost_events, list) 必然失败 → 这些课程的 freshness boost 静默失效。复现:python3 -c "from misakanet import freshness as F; print(F._extract_frontmatter(open('<file>').read())['freshness_boosts'])"
  • misakanet/cli/heal.py:171 — 原始日志写进仓库(high,已核实代码):--write 时 f.write(raw_log) 把最多 200 行任意 agent 输出落到 tests/fixtures/**/unmatched_<hash>.log。文件名是签名哈希,但内容是原始日志,与"原始日志可能含凭据/PII,绝不自动落盘"的模块自述直接冲突,且距 git add 一步。建议只写匹配到的签名 + 前 N 行。
  • misakanet/cli/remote.py:45 — 远端 path 直接拼路径(high,已核实代码):repo_root / item.get("path"),pathlib 下绝对路径会顶掉左侧 → 任何 /etc/passwd 都会成为 filepath;../ 亦可越界。加一个 resolve 后的包含性检查即可。
  • misakanet/cli/remote.py — 过滤参数被静默忽略(high):mode / domain / status_filter / tags_filter 声明了但从未使用,调用方拿到的是未过滤语料。要么接线,要么删参数。
  • misakanet/profile.py — profile.json / .quota.json 的 read-modify-write 竞争(high):原子 rename 只防半写,不防丢更新;并发调用会互相覆盖计数。
  • misakanet/freshness.py — 类型强制与异常(high×2):fallback 解析把非布尔值全变字符串;freshness_boosts 里出现非 str/dict 元素会 AttributeError 打断整篇课程的 freshness 计算。

全部条目(critical + high)

installer + 7 core scripts

  • packages/misakanet-setup/bin/misakanet-setup.mjs:210 [high] The outer try/catch returns {} for any failure (network error, abort, JSON parse error, HTTP non-2xx). Both verify() (gates on tools.length > 0) and ensureIdentity() (gates on the token-regex match) make decisions on thi
    ✅ 已核实并已修(fix(setup): keep the first backup, tighten the Hermes token file, and stop two silent failures #1774):mcpRequest 静默吞错
  • packages/misakanet-setup/bin/misakanet-setup.mjs:115 [high] HOME is fully attacker-controllable via --home (resolve(valueOf('--home', homedir())) with no validation), and writeText does mkdirSync(dirname(path), { recursive: true }). ensureIdentity then writes the MCP bearer token
  • packages/misakanet-setup/bin/misakanet-setup.mjs:332 [high] locateHook() returns the first candidate file whose contents include the literal string MisakaNet, and that text is then written to ~/.misakanet-agent/hook.mjs and wired into Claude Code's UserPromptSubmit and `PostToolUse

其余各级:{'high': 3, 'medium': 4, 'low': 2}

核心脚本(7 文件)

  • packages/misakanet-setup/scripts/copy-hook.mjs:39 [high] No try/catch around fs operations - if any copy fails mid-script the tarball ships with a partially built voice hook (player copied but only some cues). A single throw from copyFileSync leaves the destination directory in an incon
  • integrations/agent-autostart/checkpoint_reminder.mjs:259 [high] Unvalidated Number() coercion of env vars silently disables the checkpoint feature. MISAKANET_CHECKPOINT_AT, MISAKANET_CHECKPOINT_EVERY, and MISAKANET_UPDATE_AFTER_DAYS are passed straight through Number(); a non-numeric
  • integrations/agent-autostart/checkpoint_reminder.mjs:219 [high] target() forwards MISAKANET_TOKEN to whatever MISAKANET_ENDPOINT contains, with no validation of the URL scheme, host, or that the token "belongs" to that origin. The big design comment correctly excludes the file token, b
  • scripts/check_provenance.py:53 [high] SSRF: EXEMPT_PATTERNS only blocks RFC1918 IPv4 (10/8, 172.16-31/12, 192.168/16, 127/8). A malicious PR can submit http://169.254.169.254/... (cloud metadata), http://100.64.x.x (CGNAT), http://0.0.0.0, or any IPv6 host (`h
    ✅ 已核实并已修(fix: security and parsing holes found by an open-code-review scan #1773):豁免只覆盖 RFC1918,元数据地址/CGNAT/IPv6 会被 CI 请求
  • scripts/check_provenance.py:150 [high] Auth-token exfiltration: GITHUB_TOKEN / GH_TOKEN is attached as a Bearer header to every fetched URL — including attacker-supplied URLs in a PR's frontmatter. There is no host allowlist for the GitHub API token; even legit
    ✅ 已核实并已修(fix: security and parsing holes found by an open-code-review scan #1773):GITHUB_TOKEN 被发给每一个外链
  • scripts/check_provenance.py:156 [high] Silent redirect following amplifies the SSRF surface: urllib.request.urlopen follows 3xx by default, so a public https://attacker.example/redir can 302 to http://169.254.169.254/latest/meta-data/... and the bearer token from
    ✅ 已核实并已修(fix: security and parsing holes found by an open-code-review scan #1773):3xx 默认跟随,可 302 到元数据地址
  • scripts/intake_auto_review.py:860 [high] Pipeline boilerplate inflates the detail score. auto_review_issue computes word_count on the raw body (line 861) and passes it to score_detail, which then awards +10/+20/+25/+30 at the 50/100/150/300 word thresholds withou
  • scripts/intake_auto_review.py:545 [high] Pipeline boilerplate inflates the format score. Every other scorer (score_completeness, score_generalization, score_verification, score_uniqueness) starts with body = strip_pipeline_boilerplate(body), but score_format
  • integrations/agent-autostart/install_misakanet_agent.py:556 [high] DSH install violates the "marker-scoped" design rule (and breaks --uninstall). The skill is rewritten by appending a ## 自启动规则 section with no misakanet:start / misakanet:end markers, but the module docstring promises: *"ever
  • integrations/agent-autostart/install_misakanet_agent.py:319 [high] install_claude crashes mid-install if settings.permissions.allow is not a list. setdefault("allow", []) returns the existing value unchanged — so when a user has "allow": "all" (or any non-list), the next line's allow.app ✅ 已核实并已修(#1773):"allow": "all"` 会导致装到一半崩
  • scripts/sync_lesson_count.py:437 [high] Non-atomic in-place writes can corrupt the very surface this script guards. write_text truncates and rewrites the file directly; a SIGKILL, OOM, or ENOSPC mid-write leaves the file truncated to a partial pattern (e.g. the openin
  • scripts/sync_lesson_count.py:311 [high] Inconsistent error handling across the three canonical readers. canonical_nodes wraps (OSError, JSONDecodeError) into ValueError so main can print a clean "count unavailable" line, but canonical_count and `canonical_doma

其余各级:{'high': 12, 'medium': 13, 'low': 7}

misakanet 包(48 文件)

  • misakanet/cli/remote.py:57 [high] Unused filter parameters: mode, domain, status_filter, and tags_filter are declared but never referenced in the function body. The remote path calls _load_remote_docs() with no filter args, and the local fallback `load
  • misakanet/cli/remote.py:37 [high] Path traversal via untrusted path from remote JSON: rel = item.get("path") is taken straight from the server response and combined as repo_root / rel. With Python's pathlib, an absolute value (e.g. /etc/passwd) on the ri
  • misakanet/cli/heal.py:167 [high] Security/PII: when --write is used, the entire raw_log (up to 200 lines of arbitrary agent output) is persisted to tests/fixtures/openclaw/unmatched_.log, not just the matched signature. This directly undercuts the audit rat
  • misakanet/graphql/schema.py:180 [high] execute_query() rebuilds the full GraphQL schema (every GraphQLObjectType, GraphQLField, GraphQLArgument) on every invocation. The docstring explicitly states this is "lazy, only called when --graphql is used", but `_build_schema(
  • misakanet/freshness.py:218 [critical] The nested-object branch is evaluated before the list-item branch, so any YAML that defines a list of dicts (e.g. the documented freshness_boosts: [{type: was_used}, {type: helpful_vote}]) is mis-parsed as a single nested obje
  • misakanet/freshness.py:243 [high] The YAML-like fallback only converts top-level true/false; every other value (integers, floats, nested booleans/ints, ISO dates) is stored via val.strip("\"'") as a string. That breaks any field where the consumer relies on
  • misakanet/freshness.py:137 [high] The boost loop assumes every element of freshness_boosts is either a str or a dict. If a malformed entry of any other type slips in (None, int, nested list, etc.), the else event.get("type") branch calls .get on a
  • misakanet/profile.py:118 [high] Read–modify–write race on profile.json / .quota.json. The atomic rename in _save / _save_quota only protects file integrity from partial writes, NOT against lost updates: increment_search, increment_lesson, `apply_referral
  • misakanet/profile.py:97 [high] _generate_code uses random.choice to mint referral codes. These codes are identity-bearing tokens (the referrer's referral_code is persisted in another node's referred_by via apply_referral and ties attribution back to a
  • misakanet/guard.py:155 [high] stderr is buffered and only surfaced to the parent AFTER proc.communicate() returns, so the wrapped process's stderr is invisible to the operator for the entire lifetime of a long-running child. For a tool whose stated purpose is
  • misakanet/scripts/draft_reminder.py:132 [high] Invisible U+200B (zero-width space) between < and filename in this Feishu help string. Anyone copy-pasting queue_hook_stats.py promote <filename> will get an unparseable command (the shell tokenises it as promote plus a se
  • misakanet/scripts/draft_reminder.py:139 [high] Same invisible U+200B zero-width space between < and filename in the second Feishu help string. Same copy-paste breakage as above; replace with a plain <file>.
  • misakanet/scripts/bulk_import_lessons.py:179 [high] cmd_init (and the parallel cmd_import, cmd_wizard, cmd_wizard_ref sites) calls filepath.write_text() without checking whether the target slug already exists. Re-running any of them with a previously-used title silently overwrites
  • misakanet/scripts/bulk_import_lessons.py:204 [high] Same silent-overwrite hazard as in cmd_init: the wrapped content is written without checking whether a lesson with the same slug already exists in staging//.
  • misakanet/scripts/bulk_import_lessons.py:330 [high] Same silent-overwrite hazard as in cmd_init: the wizard-generated content is written without checking whether the slug is already taken.
  • misakanet/scripts/bulk_import_lessons.py:403 [high] Same silent-overwrite hazard as in cmd_init, now applied to reference/ documents — wizard-ref writes to a fixed PROJECT_ROOT/reference/ path with no existence check.
  • misakanet/scripts/queue_hook_stats.py:341 [high] Argv IndexError when --category/--hit/--error is the last token. sys.argv[sys.argv.index("--category") + 1] raises IndexError if the flag has no following value. The --node loop above correctly guards with `i + 1 < len(sys.argv)
  • misakanet/scripts/queue_hook_stats.py:144 [high] Path traversal via draft_name in cmd_promote/cmd_reject. The argument comes straight from argv[2] and is joined into drafts_dir and used as a target filename; values like ../../tmp/evil or an absolute path escape ~/.hermes/les
  • misakanet/scripts/queue_hook_stats.py:93 [high] Markdown / YAML injection via the --error field. Both _fill_template and _generate_draft interpolate error_msg directly inside a triple-backtick fenced block; an error payload that contains ``` closes the fence and can append ar
  • misakanet/scripts/clean_pipeline.py:324 [high] Filename collision causes silent data loss: two lessons sharing or fuzzy-matching titles produce the same slug and tempfile path; the second tmpfile.write_text overwrites the first before queue_lesson.py --file is invoked, so
  • misakanet/scripts/clean_pipeline.py:246 [high] Dedup printout lies about which lesson is kept: only j is added to merged, so the final keep_indices = set(range(n)) - {j ...} always retains the earlier-indexed lesson (i) regardless of body length. The `keep = i if len(l
  • misakanet/scripts/clean_pipeline.py:192 [high] Quality gate is purely cosmetic: the rejected counter is incremented and warnings printed, but lesson["fm"]["status"] is never set to "rejected", so flagged lessons flow unchanged into step_classify and step_output. The
  • misakanet/search/engine.py:296 [high] Concurrency: _L1_CACHE is a module-level mutable dict mutated on every call to _search_cached (insert, length-check, dict-iteration for eviction) but _CACHE_LOCK is never acquired here. Concurrent HTTP/MCP requests (the same
  • misakanet/search/engine.py:179 [high] Concurrency/data integrity: _L2_CONN is created with check_same_thread=False so multiple threads share it, but _CACHE_LOCK is only acquired inside _l2() around the initial CREATE/ALTER TABLE. _load_docs_cached then perfo
  • misakanet/search/engine.py:296 [high] Correctness: the L1 cache key only fingerprints the first 100 documents (docs[:100]). Any add/edit/delete to the 101st document or later leaves the fingerprint unchanged, so the L1 cache will keep returning stale rankings. Use t
  • misakanet/search/embeddings.py:79 [critical] The dev-fallback branch calls hashlib.sha256(...) but hashlib is never imported in this module (the only imports in the file are numpy and sentence_transformers, both done lazily inside functions). The first time this fall
  • misakanet/search/embeddings.py:68 [high] When encoder.encode(...) raises, the exception is caught and logged via logging.error(...) without exc_info=True, and the subsequent RuntimeError is not chained with raise ... from e. The original traceback is discarded,
  • misakanet/server/handlers/memory_context.py:32 [high] The BM25 branch builds result dicts with only title/path/score/domain/status, omitting the problem and fix fields. The downstream formatter (lines ~48-52) silently drops the Problem/Fix lines whenever either is missi
  • misakanet/scripts/inject_to_claude.py:98 [high] Silently no-op flag: the module docstring and --clean usage both advertise "注入 lessons + 清理过期条目", but inject_to_claudemd never references its clean parameter — every --clean invocation behaves identically to the no-flag ru
  • misakanet/scripts/inject_to_claude.py:60 [high] Fragile YAML detection: the parser toggles in_yaml on every standalone --- line. If a lesson body contains a horizontal rule (---) or any other ----only line after the YAML frontmatter, the loop silently re-enters "in YAML
  • misakanet/server/handlers/status.py:47 [high] _save_registration swallows every exception with bare pass. When persistence fails, a token is still returned to the caller, but it is never written to the usage_meter. The next call with the same client_id will then find no
  • misakanet/server/handlers/status.py:103 [high] TOCTOU race between _registered_tokens().get(node_id) and _save_registration(...). The check ("does a token exist for this node_id?") and the write ("save the new registration") are not atomic. Two concurrent calls with the sa
  • misakanet/server/handlers/search.py:79 [high] Lazy-init race on module-global _SEARCH_STATE: two concurrent callers can both observe None and both invoke _init_search(), duplicating index loading and leaving the global in a non-deterministic state. Use a lock (threading.Lock)
  • misakanet/server/protocol.py:178 [high] Unhandled exceptions in the stdio loop crash the entire MCP server. The try/except only guards json.loads(line); any exception raised by handle_request or any handler (e.g., bad params type, handler bug) propagates out of th
  • misakanet/server/resources.py:136 [high] Return-type contract violation: handle_resources_read is annotated -> dict, and every other branch (lessons index, protocol/overview, docs/*, lessons/{id}, unknown URI) returns a dict — including {"error": ...} on failure —
  • misakanet/tools/dashboard.py:246 [high] do_GET has no exception handling around the DB read and HTML render. If read_dashboard_data or render_dashboard_html raises (e.g. database locked/corrupted, disk error, permission issue), send_response/end_headers are neve
  • misakanet/tools/telemetry_pipeline.py:150 [high] Blocking synchronous SQLite write inside async emit(): when the queue is full the fallback path calls self._sync_write([event]) directly on the event loop thread. emit() is on the search hot path, so every queue-overflow eve
  • misakanet/tools/langchain_tool.py:331 [high] The Task returned by asyncio.ensure_future is dropped immediately; if pipeline.emit raises (e.g., its QueueFull sync-write fallback also fails), the exception is swallowed by the loop's default exception handler and never propagat
  • misakanet/tools/langchain_tool.py:328 [high] asyncio.get_event_loop() is deprecated when called outside a running loop (Python 3.10+) and on 3.12+ raises DeprecationWarning / RuntimeError. Because _arun runs _run inside asyncio.to_thread's worker thread — which has no event
  • misakanet/scripts/sync_lessons.sh: [critical] CRITICAL: --cc mode never writes the lesson content into CLAUDE.md. After deleting the old marker block (if any) the script only appends a blank line in the else branch and prints "CLAUDE.md updated", but the opening marker, the
  • misakanet/scripts/sync_lessons.sh: [high] Fragile sed range deletion: sed -i '/A/,/B/{//!d;}' relies on //!d matching against "the last regex" inside the range, whose behavior differs subtly between GNU sed and BSD/macOS sed. In practice the opening or closing marker
  • misakanet/scripts/sync_lessons.sh:30 [high] Hard-coded git pull --ff-only origin main assumes the remote is literally origin and the branch is main. Forks, custom remote names, feature-branch checkouts, shallow clones, or a master branch all break the pull. The `||
  • misakanet/scripts/draft_reminder.sh:59 [high] JSON injection from unescaped user-controlled TITLE. $TITLE is parsed straight from markdown frontmatter via grep/sed and concatenated into the JSON body, with no JSON-escape step. A title containing ", \, or a control
  • misakanet/scripts/draft_reminder.sh:68 [high] Silent webhook delivery failure + set -e interaction. curl -s ... > /dev/null 2>&1 swallows both stdout and stderr, and with set -e a non-zero exit from curl aborts the script before the success log line (`echo " → 飞书提醒已发送"
  • misakanet/scripts/lesson_watcher.sh:27 [high] PID-file lifecycle is broken on error paths. (1) The kill -0 check + echo $$ > "$PID_FILE" is a TOCTOU race; two concurrent starts can both pass the check and clobber each other's PID. (2) trap cleanup SIGINT SIGTERM does no
  • misakanet/scripts/lesson_watcher.sh:73 [high] Self-triggering recursion / no debounce. inotifywait -r watches $LESSONS_DIR, and git pull --ff-only writes into exactly that directory. Every successful pull fires more close_write/moved_to events that re-enter the loop
  • misakanet/scripts/lesson_watcher.sh:76 [high] event=$(...) && { ... } || { continue; } swallows real failures and 2>/dev/null hides diagnostic output. Any non-zero exit inside the success block (e.g. tee -a failing because /tmp is full or the log was rotated, or `inot

其余各级:{'low': 42, 'medium': 71, 'high': 44, 'critical': 3}

复现

npm install --prefix /tmp/ocr-prefix @alibaba-group/open-code-review
export HOME=/tmp/ocr-home    # 配置写在 $HOME/.opencodereview/config.json
ocr config set provider minimax
ocr config set providers.minimax.api_key "$KEY"
ocr config set providers.minimax.url https://api.minimax.chat/v1
ocr scan --path misakanet --format json --output /tmp/ocr-pkg.json --max-tokens-budget 3000000


This repo is using Opire - what does it mean? 👇
💵 Everyone can add rewards for this issue commenting /reward 100 (replace 100 with the amount).
🕵️‍♂️ If someone starts working on this issue to earn the rewards, they can comment /try to let everyone know!
🙌 And when they open the PR, they can comment /claim #1775 either in the PR description or in a PR's comment.

🪙 Also, everyone can tip any user commenting /tip 20 @Ikalus1988 (replace 20 with the amount, and @Ikalus1988 with the user to tip).

📖 If you want to learn more, check out our documentation.

Activity

  1. github-actions commented on Oct 17, 2026

    @github-actions
    Contributor

    This issue has been inactive for 30 days. It is still open — an issue here is not closed for being
    quiet, and the backlog is worked through when someone has the context to act on it.

  2. zsxh1990 commented on Oct 18, 2026

    @zsxh1990
    Contributor

    /claim

    Taking the verified severe items in this issue (the ✅ list — freshness_boosts misparse, heal.py raw-log write, remote.py path join + silently-ignored filters, profile.py RMW race, freshness.py type coercion). One PR for this issue.

    Plan, matching the issue's own criteria:

    • each item fixed with the issue's stated remedy, not a rewrite of the module
    • a regression test per item that fails before / passes after (the freshness_boosts one can assert on the exact repro command in the issue body)
    • scope limited to the ✅ items — I will not touch the「未核实」list
    • every commit Signed-off-by: (DCO)

    I'll note here before pushing anything, and open the PR with Fixes #1775 once the tests are green.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions