graphlemon: auto-commit uncommitted changes for daef8910-e6a7-4060-babd-dd736e6f1a79

graphlemon
1 parent 7674e4eb
Showing 5 changed files with 201 additions and 0 deletions
frontend/app.js
src/backend_server.py
src/skill_query.py
src/test_backend_server.py
src/test_skill_query.py
@@ -1738,6 +1738,9 @@ function renderSkillAuditLog(skillId) {
       <div class="breadcrumb"><a href="#/skills">技能库</a> / ${escapeHtml(skillId)}</div>
       <h1>${escapeHtml(skillId)} 变更审计</h1>
       <div id="skill_audit_error" class="error-banner" style="display:none;"></div>
+      <h2>Skill 内容</h2>
+      <div id="skill_content"><div class="hint">加载中…</div></div>
+      <h2>迭代记录</h2>
       <div id="skill_audit_list"><div class="hint">加载中…</div></div>
     </div>
   `;
@@ -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 = '<div class="hint">' + (body.error === "CONTENT_MISSING" ? "该 Skill 的真实文件尚未迁移" : "该 Skill 不存在") + "</div>";
+      return;
+    }
+    const skill = await resp.json();
+    const supportHtml = skill.support_files.length
+      ? '<div class="hint">支持文件：' + skill.support_files.map(escapeHtml).join("、") + "</div>"
+      : "";
+    contentEl.innerHTML = supportHtml + '<pre class="code">' + escapeHtml(skill.content) + "</pre>";
+  });
   async function refresh() {
     const resp = await fetch("/api/skills/" + encodeURIComponent(skillId) + "/audit");
@@ -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]
@@ -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/<skill_id>/). 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 <before> <after>` -- skill_update.py's
     _write_and_commit makes every iteration an atomic single-file commit,
@@ -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 /
@@ -5,6 +5,7 @@ graphlemon&#39;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")