Stop building generated code from unsanitized free text in skill instructions

graphify/cli.py

@@ -1461,22 +1461,46 @@ def dispatch_command(cmd: str) -> None:
     elif cmd == "save-result":
         # graphify save-result --question Q --answer A [--type T] [--nodes N1 N2 ...]
         #                      [--outcome useful|dead_end|corrected] [--correction TEXT]
+        #
+        # --question-file/--answer-file/--correction-file read the value from a
+        # file instead of a command-line argument: skill instructions that build
+        # this command from free text (the user's verbatim question, an LLM's
+        # generated answer, of unbounded length and content) must not substitute
+        # that text directly into a shell command string, since embedded quotes,
+        # backticks, or $() would corrupt or escape the command entirely
+        # (#3439). Writing the value to a file first has no such injection
+        # surface at all.
         import argparse as _ap
 
         p = _ap.ArgumentParser(prog="graphify save-result")
-        p.add_argument("--question", required=True)
+        p.add_argument("--question", default=None)
+        p.add_argument("--question-file", dest="question_file", default=None)
         p.add_argument("--answer", default=None)
         p.add_argument("--answer-file", dest="answer_file", default=None)
         p.add_argument("--type", dest="query_type", default="query")
         p.add_argument("--nodes", nargs="*", default=[])
+        p.add_argument("--nodes-file", dest="nodes_file", default=None)
         p.add_argument("--outcome", choices=("useful", "dead_end", "corrected"), default=None)
         p.add_argument("--correction", default=None)
+        p.add_argument("--correction-file", dest="correction_file", default=None)
         p.add_argument("--memory-dir", default=str(Path(_GRAPHIFY_OUT) / "memory"))
         opts = p.parse_args(sys.argv[2:])
+        if opts.question_file:
+            opts.question = Path(opts.question_file).read_text(encoding="utf-8").strip()
+        elif opts.question is None:
+            p.error("--question or --question-file is required")
         if opts.answer_file:
             opts.answer = Path(opts.answer_file).read_text(encoding="utf-8").strip()
-        elif not opts.answer:
+        elif opts.answer is None:
             p.error("--answer or --answer-file is required")
+        if opts.correction_file:
+            opts.correction = Path(opts.correction_file).read_text(encoding="utf-8").strip()
+        if opts.nodes_file:
+            opts.nodes = [
+                line.strip()
+                for line in Path(opts.nodes_file).read_text(encoding="utf-8").splitlines()
+                if line.strip()
+            ]
         from graphify.ingest import save_query_result as _sqr
 
         out = _sqr(
@@ -1942,32 +1966,84 @@ def dispatch_command(cmd: str) -> None:
             print(format_diagnostic_report(summary))
 
     elif cmd == "add":
-        if len(sys.argv) < 3:
-            print(
-                "Usage: graphify add <url> [--author Name] [--contributor Name] [--dir ./raw]",
-                file=sys.stderr,
-            )
+        # --from-file reads {"url": ..., "author": ..., "contributor": ..., "dir": ...}
+        # (author/contributor/dir optional) instead of taking url/--author/
+        # --contributor as command-line text: skill instructions that build this
+        # command from a user-supplied URL and name have no way to shell-quote
+        # values they cannot predict the content of, so substituting them
+        # directly into a command string risks corrupting or escaping it
+        # (#3439). Writing the payload to a file first has no such injection
+        # surface -- only the file's own (agent-controlled) path is a shell
+        # argument.
+        args = sys.argv[2:]
+        from_file = None
+        from_file_requested = False
+        for i, a in enumerate(args):
+            if a == "--from-file":
+                from_file_requested = True
+                if i + 1 < len(args):
+                    from_file = args[i + 1]
+                break
+        if from_file_requested and not from_file:
+            print("error: --from-file requires a path argument", file=sys.stderr)
             sys.exit(1)
+        if from_file:
+            try:
+                payload = json.loads(Path(from_file).read_text(encoding="utf-8"))
+            except (OSError, json.JSONDecodeError) as exc:
+                print(f"error: could not read --from-file payload: {exc}", file=sys.stderr)
+                sys.exit(1)
+            if not isinstance(payload, dict):
+                print(
+                    "error: --from-file payload must be a JSON object with a "
+                    "'url' key, not " + type(payload).__name__,
+                    file=sys.stderr,
+                )
+                sys.exit(1)
+            try:
+                url = payload["url"]
+            except KeyError:
+                print("error: --from-file payload is missing required key 'url'", file=sys.stderr)
+                sys.exit(1)
+            author = payload.get("author")
+            contributor = payload.get("contributor")
+            raw_dir = payload.get("dir")
+            if raw_dir is not None and not isinstance(raw_dir, str):
+                print(
+                    "error: --from-file payload 'dir' must be a string, not "
+                    + type(raw_dir).__name__,
+                    file=sys.stderr,
+                )
+                sys.exit(1)
+            target_dir = Path(raw_dir or "raw")
+        else:
+            if len(sys.argv) < 3:
+                print(
+                    "Usage: graphify add <url> [--author Name] [--contributor Name] "
+                    "[--dir ./raw] | graphify add --from-file payload.json",
+                    file=sys.stderr,
+                )
+                sys.exit(1)
+            url = sys.argv[2]
+            author = None
+            contributor = None
+            target_dir = Path("raw")
+            args = sys.argv[3:]
+            i = 0
+            while i < len(args):
+                if args[i] == "--author" and i + 1 < len(args):
+                    author = args[i + 1]
+                    i += 2
+                elif args[i] == "--contributor" and i + 1 < len(args):
+                    contributor = args[i + 1]
+                    i += 2
+                elif args[i] == "--dir" and i + 1 < len(args):
+                    target_dir = Path(args[i + 1])
+                    i += 2
+                else:
+                    i += 1
         from graphify.ingest import ingest as _ingest
 
-        url = sys.argv[2]
-        author: str | None = None
-        contributor: str | None = None
-        target_dir = Path("raw")
-        args = sys.argv[3:]
-        i = 0
-        while i < len(args):
-            if args[i] == "--author" and i + 1 < len(args):
-                author = args[i + 1]
-                i += 2
-            elif args[i] == "--contributor" and i + 1 < len(args):
-                contributor = args[i + 1]
-                i += 2
-            elif args[i] == "--dir" and i + 1 < len(args):
-                target_dir = Path(args[i + 1])
-                i += 2
-            else:
-                i += 1
         try:
             saved = _ingest(url, target_dir, author=author, contributor=contributor)
             print(f"Saved to {saved}")

graphify/skills/agents/references/add-watch.md

@@ -6,25 +6,35 @@ Load this when the user ran `/graphify add <url>` or passed `--watch`. Neither i
 
 Fetch a URL and add it to the corpus, then update the graph.
 
+The URL and any author/contributor name are free text you do not control the
+content of - do not build a command or inline script by substituting them
+into a string; an embedded quote or shell character corrupts or escapes it.
+Reserve a unique file path first - a fixed, shared filename risks a
+concurrent graphify session overwriting or reading a stale payload:
+
+```bash
+mktemp /tmp/graphify_add_payload.XXXXXX.json
+```
+
+Using your file-write tool (not a shell heredoc, which has the same
+quoting problem one level down), write a JSON file with those values to
+the path that command printed, then pass only that path - not its content
+- to `graphify add`:
+
+```json
+{"url": "URL", "author": "AUTHOR", "contributor": "CONTRIBUTOR", "dir": "./raw"}
+```
+
+Replace `URL` with the actual URL, `AUTHOR` with the user's name if
+provided (omit the key entirely if not), `CONTRIBUTOR` likewise, then run:
+
 ```bash
-$(cat graphify-out/.graphify_python) -c "
-import sys
-from graphify.ingest import ingest
-from pathlib import Path
-
-try:
-    out = ingest('URL', Path('./raw'), author='AUTHOR', contributor='CONTRIBUTOR')
-    print(f'Saved to {out}')
-except ValueError as e:
-    print(f'error: {e}', file=sys.stderr)
-    sys.exit(1)
-except RuntimeError as e:
-    print(f'error: {e}', file=sys.stderr)
-    sys.exit(1)
-"
+$(cat graphify-out/.graphify_python) -m graphify add --from-file PAYLOAD_PATH
 ```
 
-Replace `URL` with the actual URL, `AUTHOR` with the user's name if provided, `CONTRIBUTOR` likewise. If the command exits with an error, tell the user what went wrong - do not silently continue. After a successful save, automatically run the `--update` pipeline on `./raw` to merge the new file into the existing graph.
+Replace `PAYLOAD_PATH` with the path `mktemp` printed. If the command exits with an error, tell the user what went wrong - do not
+silently continue. After a successful save, automatically run the `--update`
+pipeline on `./raw` to merge the new file into the existing graph.
 
 Supported URL types (auto-detected):
 - YouTube / any video URL → audio downloaded via yt-dlp, transcribed to `.txt` on next run (requires `pip install 'graphifyy[video]'`)

graphify/skills/agents/references/query.md

@@ -165,15 +165,34 @@ print(output)
 
 Replace `QUESTION` with the **expanded** query string, `MODE` with `bfs` or `dfs`, and `BUDGET` with the token budget (default `2000`, or whatever `--budget N` specifies). Then answer based on the subgraph output above, using only what the graph contains.
 
-After writing the answer, save it back into the graph so it improves future queries. Include the expanded tokens inside the `--answer` text (e.g. `"Expanded from original query via vocab: [tokens]. Then traversed..."`) so the next `--update` extracts the expansion history as a graph node:
+After writing the answer, save it back into the graph so it improves future queries. Include the expanded tokens inside the answer text (e.g. `"Expanded from original query via vocab: [tokens]. Then traversed..."`) so the next `--update` extracts the expansion history as a graph node.
+
+The question and answer are free text you do not control the content of,
+and the node labels you cite can also come from extracted document content
+- a quote, backtick, or `$()` embedded in any of the three corrupts or
+escapes a command it's substituted into. Reserve three unique file paths
+first - a fixed, shared filename risks a concurrent graphify session
+overwriting or reading a stale value:
+
+```bash
+mktemp /tmp/graphify_question.XXXXXX
+mktemp /tmp/graphify_answer.XXXXXX
+mktemp /tmp/graphify_nodes.XXXXXX
+```
+
+Using your file-write tool, write the user's verbatim question to the path
+the first command printed, your full answer text (containing the
+expanded-token trace) to the path the second one printed, and the node
+labels you cited to the path the third one printed - one label per line -
+then pass those exact paths - not their content - on the command line:
 
 ```bash
-$(cat graphify-out/.graphify_python) -m graphify save-result --question "ORIGINAL_QUESTION" --answer "ANSWER" --type query --nodes NODE1 NODE2
+$(cat graphify-out/.graphify_python) -m graphify save-result --question-file QUESTION_PATH --answer-file ANSWER_PATH --type query --nodes-file NODES_PATH
 ```
 
-Replace `ORIGINAL_QUESTION` with the user's verbatim question, `ANSWER` with your full answer text (containing the expanded-token trace), `NODE1 NODE2` with the list of node labels you cited. This closes the feedback loop: the next `--update` will extract this Q&A as a node in the graph.
+Replace `QUESTION_PATH`/`ANSWER_PATH`/`NODES_PATH` with the paths `mktemp` printed. This closes the feedback loop: the next `--update` will extract this Q&A as a node in the graph.
 
-**Work memory (self-improving loop).** Add an `--outcome` so future sessions learn from this one — append `--outcome useful|dead_end|corrected` to the `save-result` command (and `--correction "the right answer"` when correcting):
+**Work memory (self-improving loop).** Add an `--outcome` so future sessions learn from this one — append `--outcome useful|dead_end|corrected` to the `save-result` command (and, when correcting, reserve one more unique path with `mktemp`, write what was right to it, and pass `--correction-file CORRECTION_PATH` the same way):
 
 - `useful` — the cited nodes answered the question well (they become *preferred sources*).
 - `dead_end` — the question/path led nowhere; don't re-derive it next time.
@@ -191,7 +210,20 @@ Find the shortest path between two named concepts in the graph. Prefer the CLI w
 graphify path "NODE_A" "NODE_B"
 ```
 
-If the CLI is unavailable, run it inline:
+If the CLI is unavailable, run it inline. The two concept names can come
+from extracted document content and so are not guaranteed free of shell
+characters - reserve a unique file path for them first (a fixed, shared
+filename risks a concurrent graphify session overwriting or reading a
+stale value):
+
+```bash
+mktemp /tmp/graphify_nodes.XXXXXX
+```
+
+Using your file-write tool, write the two concept names to that path, one
+per line (first line the source concept, second line the target), then
+run the traversal reading them back from the file instead of substituting
+them into the script:
 
 ```bash
 $(cat graphify-out/.graphify_python) -c "
@@ -203,8 +235,7 @@ from pathlib import Path
 data = json.loads(Path('graphify-out/graph.json').read_text(encoding='utf-8'))
 G = json_graph.node_link_graph(data, edges='links')
 
-a_term = 'NODE_A'
-b_term = 'NODE_B'
+a_term, b_term = Path('NODES_PATH').read_text(encoding='utf-8').strip().splitlines()
 
 def find_node(term):
     term = term.lower()
@@ -241,12 +272,25 @@ except nx.NodeNotFound as e:
 "
 ```
 
-Replace `NODE_A` and `NODE_B` with the actual concept names from the user. Then explain the path in plain language - what each hop means, why it's significant.
+Replace `NODES_PATH` with the path `mktemp` printed. Then explain the path in plain language - what each hop means, why it's significant.
 
-After writing the explanation, save it back:
+After writing the explanation, save it back. Reserve two more unique file
+paths for the question and answer text - the same free text risk as
+`/graphify query` above (a fixed, shared filename risks a concurrent
+graphify session overwriting or reading a stale value):
 
 ```bash
-$(cat graphify-out/.graphify_python) -m graphify save-result --question "Path from NODE_A to NODE_B" --answer "ANSWER" --type path_query --nodes NODE_A NODE_B
+mktemp /tmp/graphify_question.XXXXXX
+mktemp /tmp/graphify_answer.XXXXXX
+```
+
+Using your file-write tool, write `Path from <source concept> to <target
+concept>` (with the actual node names) to the first path and the
+explanation to the second, then pass those paths and the same node-labels
+file reserved above, the same way as for `/graphify query` above:
+
+```bash
+$(cat graphify-out/.graphify_python) -m graphify save-result --question-file QUESTION_PATH --answer-file ANSWER_PATH --type path_query --nodes-file NODES_PATH
 ```
 
 ---
@@ -259,7 +303,19 @@ Give a plain-language explanation of a single node - everything connected to it.
 graphify explain "NODE_NAME"
 ```
 
-If the CLI is unavailable, run it inline:
+If the CLI is unavailable, run it inline. The concept name can come from
+extracted document content and so is not guaranteed free of shell
+characters - reserve a unique file path for it first (a fixed, shared
+filename risks a concurrent graphify session overwriting or reading a
+stale value):
+
+```bash
+mktemp /tmp/graphify_nodes.XXXXXX
+```
+
+Using your file-write tool, write the concept name to that path, then run
+the lookup reading it back from the file instead of substituting it into
+the script:
 
 ```bash
 $(cat graphify-out/.graphify_python) -c "
@@ -271,7 +327,7 @@ from pathlib import Path
 data = json.loads(Path('graphify-out/graph.json').read_text(encoding='utf-8'))
 G = json_graph.node_link_graph(data, edges='links')
 
-term = 'NODE_NAME'
+term = Path('NODES_PATH').read_text(encoding='utf-8').strip()
 term_lower = term.lower()
 
 # Find best matching node
@@ -302,10 +358,23 @@ for neighbor in G.neighbors(nid):
 "
 ```
 
-Replace `NODE_NAME` with the concept the user asked about. Then write a 3-5 sentence explanation of what this node is, what it connects to, and why those connections are significant. Use the source locations as citations.
+Replace `NODES_PATH` with the path `mktemp` printed. Then write a 3-5 sentence explanation of what this node is, what it connects to, and why those connections are significant. Use the source locations as citations.
+
+After writing the explanation, save it back. Reserve two more unique file
+paths for the question and answer text - the same free text risk as
+`/graphify query` above (a fixed, shared filename risks a concurrent
+graphify session overwriting or reading a stale value):
+
+```bash
+mktemp /tmp/graphify_question.XXXXXX
+mktemp /tmp/graphify_answer.XXXXXX
+```
 
-After writing the explanation, save it back:
+Using your file-write tool, write `Explain` followed by the actual node
+name to the first path and the explanation to the second, then pass those
+paths and the same node-label file reserved above, the same way as for
+`/graphify query` above:
 
 ```bash
-$(cat graphify-out/.graphify_python) -m graphify save-result --question "Explain NODE_NAME" --answer "ANSWER" --type explain --nodes NODE_NAME
+$(cat graphify-out/.graphify_python) -m graphify save-result --question-file QUESTION_PATH --answer-file ANSWER_PATH --type explain --nodes-file NODES_PATH
 ```