From a20bf50842fddd487aa96682f91b75216e95dbd3 Mon Sep 17 00:00:00 2001 From: graphlemon Date: Wed, 5 Aug 2026 13:28:11 +0800 Subject: [PATCH] graphlemon: auto-commit uncommitted changes for daef8910-e6a7-4060-babd-dd736e6f1a79 --- frontend/app.js | 21 +++++++++++++++++++++ src/backend_server.py | 20 ++++++++++++++++++++ src/skill_query.py | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/test_backend_server.py | 17 +++++++++++++++++ src/test_skill_query.py | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 201 insertions(+), 0 deletions(-) diff --git a/frontend/app.js b/frontend/app.js index 5a7da1d..da24c1e 100644 --- a/frontend/app.js +++ b/frontend/app.js @@ -1738,6 +1738,9 @@ function renderSkillAuditLog(skillId) {
技能库 / ${escapeHtml(skillId)}

${escapeHtml(skillId)} 变更审计

+

Skill 内容

+
加载中…
+

迭代记录

加载中…
`; @@ -1746,6 +1749,24 @@ function renderSkillAuditLog(skillId) { function mountSkillAuditLog(skillId) { const listEl = document.getElementById("skill_audit_list"); const errorEl = document.getElementById("skill_audit_error"); + const contentEl = document.getElementById("skill_content"); + + // Independent of the iteration-history panel below (a skill can have real + // content with zero skill_iteration rows -- that's the normal post- + // migration state) -- fetched and rendered separately, side by side, not + // as an either/or with refresh()'s own list. + fetch("/api/skills/" + encodeURIComponent(skillId) + "/content").then(async (resp) => { + if (resp.status !== 200) { + const body = await resp.json(); + contentEl.innerHTML = '
' + (body.error === "CONTENT_MISSING" ? "该 Skill 的真实文件尚未迁移" : "该 Skill 不存在") + "
"; + return; + } + const skill = await resp.json(); + const supportHtml = skill.support_files.length + ? '
支持文件：' + skill.support_files.map(escapeHtml).join("、") + "
" + : ""; + contentEl.innerHTML = supportHtml + '
' + escapeHtml(skill.content) + "
"; + }); async function refresh() { const resp = await fetch("/api/skills/" + encodeURIComponent(skillId) + "/audit"); diff --git a/src/backend_server.py b/src/backend_server.py index 2b442d4..470de0d 100644 --- a/src/backend_server.py +++ b/src/backend_server.py @@ -195,6 +195,10 @@ class _Handler(BaseHTTPRequestHandler): skill_id = parsed.path[len(_SKILLS_PREFIX):-len("/audit")] self._handle_skill_audit_log(skill_id) return + if parsed.path.startswith(_SKILLS_PREFIX) and parsed.path.endswith("/content"): + skill_id = parsed.path[len(_SKILLS_PREFIX):-len("/content")] + self._handle_skill_content(skill_id) + return if parsed.path == "/api/deployments": self._handle_deployment_list() return @@ -704,6 +708,22 @@ class _Handler(BaseHTTPRequestHandler): else: self._send_json(200, result) + def _handle_skill_content(self, skill_id): + """c_skill_content (bug-fix Unit, no MVCP operation_id -- same + "unowned gap" precedent as c_artifact_get before U-025). Real + SKILL.md content off disk, alongside (not instead of) the existing + /audit iteration history -- v_skill_audit_log's page renders both + independently.""" + try: + with self.server.conn_lock: + result = skill_query.get_skill_content(self.server.conn, skill_id, repo_path=self.server.skill_repo_path) + except skill_query.SkillNotFoundError: + self._send_json(404, {"error": "NOT_FOUND"}) + except skill_query.SkillContentMissingError: + self._send_json(404, {"error": "CONTENT_MISSING"}) + else: + self._send_json(200, result) + def _handle_skill_diff(self, query_string): params = urllib.parse.parse_qs(query_string) before = params.get("before", [None])[0] diff --git a/src/skill_query.py b/src/skill_query.py index 395e987..326727b 100644 --- a/src/skill_query.py +++ b/src/skill_query.py @@ -10,6 +10,8 @@ rather than inventing a second way to locate it. import os import subprocess +from src import skill_update + _REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) @@ -52,6 +54,67 @@ def get_skill_audit_log(conn, skill_id): ] +class SkillContentMissingError(Exception): + """CONTENT_MISSING -- get_skill_content on a skill row whose real + SKILL.md file isn't present on disk. Distinct from SkillNotFoundError + (no such skill row at all): a skill row can legitimately exist with its + real file not-yet-migrated, and callers (v_skill_audit_log's new "Skill + 内容" panel) need to tell the two apart to show the right message.""" + + +def get_skill_content(conn, skill_id, repo_path=None): + """v_skill_audit_log's new "Skill 内容" panel (bug fix: this page used to + show only skill_iteration history -- correct on its own terms, but + silent about the real SKILL.md content a skill migration already wrote + to disk under skills//). Independent of get_skill_audit_log: + a skill can have real content with zero iterations, which is exactly + the post-migration, pre-any-self-iteration state today. + + skill_id is user-controlled (comes straight off the URL), but it is + never spliced into a filesystem path itself -- the lookup below is a + parameterized query, and the actual on-disk path comes from the + trusted `skill.file_path` DB column. Still applies two-layer + path-traversal protection on that DB value: reuse + skill_update._skill_file_path() for the same SKILL.md-suffix + resolution c_skill_update/c_skill_rollback already rely on, then + realpath+containment-check the result against the repo's skills/ root + before ever opening it -- defense in depth against a future schema/data + bug turning this read path into an arbitrary-file-read, not a fix for + an exploit that's live today. + """ + row = conn.execute("SELECT file_path FROM skill WHERE skill_id = ?", (skill_id,)).fetchone() + if row is None: + raise SkillNotFoundError(skill_id) + + repo_root = repo_path or _REPO_ROOT + skills_root = os.path.realpath(os.path.join(repo_root, "skills")) + rel_path = skill_update._skill_file_path(row[0]) + abs_path = os.path.realpath(os.path.join(repo_root, rel_path)) + if os.path.commonpath([abs_path, skills_root]) != skills_root: + # Layer 1: DB-stored file_path resolves outside skills/ entirely. + raise SkillContentMissingError(skill_id) + + try: + with open(abs_path, "r", encoding="utf-8", errors="replace") as f: + content = f.read() + except OSError: + # Layer 2: within skills/ but the file itself isn't there (not-yet- + # migrated skill, or a stale DB row). + raise SkillContentMissingError(skill_id) + + skill_dir = os.path.dirname(abs_path) + support_files = [] + for dirpath, _dirnames, filenames in os.walk(skill_dir): + for name in filenames: + full_path = os.path.realpath(os.path.join(dirpath, name)) + if full_path == abs_path: + continue + support_files.append(os.path.relpath(full_path, skill_dir)) + support_files.sort() + + return {"skill_id": skill_id, "content": content, "support_files": support_files} + + def get_diff(before, after, repo_path=None): """c_skill_diff: `git diff ` -- skill_update.py's _write_and_commit makes every iteration an atomic single-file commit, diff --git a/src/test_backend_server.py b/src/test_backend_server.py index c156be1..5dc6071 100644 --- a/src/test_backend_server.py +++ b/src/test_backend_server.py @@ -1521,6 +1521,23 @@ class SkillRouteTest(unittest.TestCase): status, body = self._post("/api/skills/skill-1/rollback", {}) self.assertEqual(status, 400) + def test_content_returns_real_file_content(self): + skill_update._write_and_commit(self.tmp.name, "skills/skill-1/SKILL.md", "real content\n", "seed skill") + status, body = self._get("/api/skills/skill-1/content") + self.assertEqual(status, 200) + self.assertEqual(body["content"], "real content\n") + self.assertEqual(body["support_files"], []) + + def test_content_nonexistent_skill_returns_404(self): + status, body = self._get("/api/skills/no-such-skill/content") + self.assertEqual(status, 404) + self.assertEqual(body["error"], "NOT_FOUND") + + def test_content_missing_file_returns_404_content_missing(self): + status, body = self._get("/api/skills/skill-1/content") + self.assertEqual(status, 404) + self.assertEqual(body["error"], "CONTENT_MISSING") + class SettingsRouteTest(unittest.TestCase): """U-030: c_concurrency_config_update / c_budget_config_update / diff --git a/src/test_skill_query.py b/src/test_skill_query.py index b97a63f..3f4b8bd 100644 --- a/src/test_skill_query.py +++ b/src/test_skill_query.py @@ -5,6 +5,7 @@ graphlemon's own -- same reasoning as test_skill_update.py). Run: python3 -m unittest src.test_skill_query -v """ +import os import subprocess import tempfile import unittest @@ -87,6 +88,85 @@ class GetSkillAuditLogTest(SkillQueryTestBase): self.assertEqual(result[0]["change_summary"], "fix A") +class GetSkillContentTest(SkillQueryTestBase): + """Bug fix: the skill detail page showed only skill_iteration history + and nothing for the real migrated SKILL.md content sitting on disk. + Uses the same tmp git repo as the rest of this file's tests -- these + file reads don't need git at all, but keeping repo_path consistent with + the other tests here avoids inventing a second fixture shape.""" + + def _write_skill_file(self, skill_id, content, extra_files=None): + skill_dir = os.path.join(self.tmp.name, "skills", skill_id) + os.makedirs(skill_dir, exist_ok=True) + with open(os.path.join(skill_dir, "SKILL.md"), "w", encoding="utf-8") as f: + f.write(content) + for rel in extra_files or []: + full = os.path.join(skill_dir, rel) + os.makedirs(os.path.dirname(full), exist_ok=True) + with open(full, "w", encoding="utf-8") as f: + f.write("support file content") + + def test_nonexistent_skill_raises_not_found(self): + with self.assertRaises(skill_query.SkillNotFoundError): + skill_query.get_skill_content(self.conn, "no-such-skill", repo_path=self.tmp.name) + + def test_returns_real_file_content(self): + self._insert_skill("s1", "coder") + self._write_skill_file("s1", "real skill content\n") + result = skill_query.get_skill_content(self.conn, "s1", repo_path=self.tmp.name) + self.assertEqual(result["skill_id"], "s1") + self.assertEqual(result["content"], "real skill content\n") + self.assertEqual(result["support_files"], []) + + def test_lists_support_files_alongside_content(self): + self._insert_skill("s1", "coder") + self._write_skill_file( + "s1", "content\n", + extra_files=["references/notes.md", "templates/t.md", "scripts/run.sh"], + ) + result = skill_query.get_skill_content(self.conn, "s1", repo_path=self.tmp.name) + self.assertEqual( + set(result["support_files"]), + {"references/notes.md", "templates/t.md", "scripts/run.sh"}, + ) + + def test_row_exists_but_file_missing_raises_content_missing(self): + self._insert_skill("s1", "coder") + with self.assertRaises(skill_query.SkillContentMissingError): + skill_query.get_skill_content(self.conn, "s1", repo_path=self.tmp.name) + + def test_file_path_already_ending_in_md_is_read_directly(self): + self._insert_skill("s1", "coder") + self.conn.execute("UPDATE skill SET file_path = ? WHERE skill_id = ?", ("skills/s1/SKILL.md", "s1")) + self.conn.commit() + self._write_skill_file("s1", "direct path content\n") + result = skill_query.get_skill_content(self.conn, "s1", repo_path=self.tmp.name) + self.assertEqual(result["content"], "direct path content\n") + + def test_path_traversal_in_stored_file_path_is_rejected(self): + """Defense-in-depth: even if skill.file_path were somehow poisoned + with a traversal sequence (not reachable via the HTTP skill_id + param today, since that only drives a parameterized WHERE clause), + the resolved path must stay confined to skills/.""" + self._insert_skill("s1", "coder") + self.conn.execute( + "UPDATE skill SET file_path = ? WHERE skill_id = ?", ("../../../../etc/passwd", "s1"), + ) + self.conn.commit() + with self.assertRaises(skill_query.SkillContentMissingError): + skill_query.get_skill_content(self.conn, "s1", repo_path=self.tmp.name) + + def test_path_traversal_via_dotdot_segments_stays_confined(self): + self._insert_skill("s1", "coder") + self.conn.execute( + "UPDATE skill SET file_path = ? WHERE skill_id = ?", + ("skills/../../outside/SKILL.md", "s1"), + ) + self.conn.commit() + with self.assertRaises(skill_query.SkillContentMissingError): + skill_query.get_skill_content(self.conn, "s1", repo_path=self.tmp.name) + + class GetDiffTest(SkillQueryTestBase): def test_diff_between_two_commits_shows_content_change(self): before_sha = skill_update._write_and_commit(self.tmp.name, "skills/s1/SKILL.md", "v1 content\n", "seed") -- libgit2 0.21.4