Skip to content

Commit 8b5adcf

Browse files
committed
fix(ide): harden OpenCode plugin for cross-platform and security
- Replace curl subprocess with Node.js http/https modules for Windows compat - Fix command injection in hook plugins (use shell:true with constant arg) - Fix JSONC comment stripping to handle inline comments - Add message.updated event handling and conditional final flag - Cap sessionState Map to prevent memory leaks in long-lived processes - Remove duplicate plugin source from CLI (delegate to server) - Validate URLs in HTTP hook plugin generation - Fix detect_hooks to not rely on Path.cwd() - Fix frontmatter parsing to only match top-level keys - Restore test coverage for session.created and message.updated - Bump HOOKS_SPEC_VERSION to 2
1 parent 58a8f51 commit 8b5adcf

4 files changed

Lines changed: 141 additions & 262 deletions

File tree

observal-server/services/ide/helpers.py

Lines changed: 77 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -202,9 +202,10 @@ def _opencode_plugin_js() -> str:
202202
// Auto-generated by `observal pull` / `observal doctor patch`
203203
// Do not edit manually - regenerated on upgrade.
204204
205-
import { execFileSync } from "child_process";
206205
import { readFileSync, existsSync } from "fs";
207206
import { join } from "path";
207+
import { request as httpRequest } from "http";
208+
import { request as httpsRequest } from "https";
208209
209210
const CONFIG_PATH = join(
210211
process.env.HOME || process.env.USERPROFILE || "~",
@@ -232,28 +233,45 @@ def _opencode_plugin_js() -> str:
232233
const body = JSON.stringify(payload);
233234
234235
try {
235-
execFileSync("curl", [
236-
"-s", "-X", "POST",
237-
"-H", "Content-Type: application/json",
238-
"-H", `Authorization: Bearer ${config.access_token}`,
239-
"--max-time", "10",
240-
"-d", body,
241-
url,
242-
], { stdio: ["pipe", "pipe", "pipe"], timeout: 12000 });
236+
const parsed = new URL(url);
237+
const reqFn = parsed.protocol === "https:" ? httpsRequest : httpRequest;
238+
const req = reqFn(url, {
239+
method: "POST",
240+
headers: {
241+
"Content-Type": "application/json",
242+
"Authorization": `Bearer ${config.access_token}`,
243+
"Content-Length": Buffer.byteLength(body),
244+
},
245+
timeout: 10000,
246+
});
247+
req.on("error", () => {});
248+
req.on("timeout", () => { req.destroy(); });
249+
req.write(body);
250+
req.end();
243251
} catch {
244252
// Non-blocking: never break the session
245253
}
246254
}
247255
248256
const sessionState = new Map();
257+
const MAX_TRACKED_SESSIONS = 50;
249258
250259
function getState(sessionId) {
251260
if (!sessionState.has(sessionId)) {
261+
// Evict oldest sessions if we exceed the cap
262+
if (sessionState.size >= MAX_TRACKED_SESSIONS) {
263+
const oldest = sessionState.keys().next().value;
264+
sessionState.delete(oldest);
265+
}
252266
sessionState.set(sessionId, { pushedMessageIds: new Set(), lineOffset: 0 });
253267
}
254268
return sessionState.get(sessionId);
255269
}
256270
271+
function cleanupSession(sessionId) {
272+
sessionState.delete(sessionId);
273+
}
274+
257275
function messagesToLines(messages) {
258276
const lines = [];
259277
for (const msg of messages) {
@@ -276,6 +294,7 @@ def _opencode_plugin_js() -> str:
276294
if (info.createdAt && typeof info.createdAt === "string") { ts = info.createdAt; }
277295
else if (info.time && typeof info.time === "object" && info.time.created) { ts = new Date(info.time.created).toISOString(); }
278296
else if (info.time && typeof info.time === "string") { ts = info.time; }
297+
else if (info.timestamp && typeof info.timestamp === "string") { ts = info.timestamp; }
279298
const line = {
280299
type: role === "user" ? "user" : "assistant",
281300
timestamp: ts,
@@ -298,9 +317,9 @@ def _opencode_plugin_js() -> str:
298317
}
299318
300319
export const ObservalPlugin = async ({ project, client, directory }) => {
301-
// Built-in agents that should NOT be tracked
302320
const BUILTIN_AGENTS = new Set(["build", "plan", "general", "explore", "scout", "compaction", "title", "summary"]);
303321
const agentSessions = new Map();
322+
const pendingPush = new Map();
304323
305324
return {
306325
event: async ({ event }) => {
@@ -309,13 +328,25 @@ def _opencode_plugin_js() -> str:
309328
const agent = event?.properties?.info?.agent || "";
310329
if (sessionId && agent && !BUILTIN_AGENTS.has(agent)) {
311330
agentSessions.set(sessionId, agent);
331+
pendingPush.set(sessionId, true);
312332
}
313333
}
334+
335+
if (event?.type === "message.updated") {
336+
const sessionId = event?.properties?.sessionID || "";
337+
if (sessionId && agentSessions.has(sessionId)) {
338+
pendingPush.set(sessionId, true);
339+
}
340+
}
341+
314342
if (event?.type === "session.idle") {
315343
const sessionId = event?.properties?.sessionID || "";
316344
if (!sessionId) return;
317345
const agent = agentSessions.get(sessionId);
318346
if (!agent) return;
347+
if (!pendingPush.get(sessionId)) return;
348+
pendingPush.delete(sessionId);
349+
319350
try {
320351
const messagesResult = await client.session.messages({ path: { id: sessionId } });
321352
const messages = messagesResult?.data || messagesResult || [];
@@ -325,14 +356,19 @@ def _opencode_plugin_js() -> str:
325356
if (newMessages.length === 0) return;
326357
const lines = messagesToLines(newMessages);
327358
if (lines.length === 0) return;
359+
const isFinal = event?.properties?.final === true || event?.properties?.reason === "completed";
328360
pushToServer({
329361
session_id: sessionId, ide: "opencode", lines, agent_id: agent,
330362
start_offset: state.lineOffset, hook_event: "session.idle",
331-
final: true, total_line_count: state.lineOffset + lines.length,
363+
final: isFinal, total_line_count: state.lineOffset + lines.length,
332364
total_offset: state.lineOffset + lines.length,
333365
});
334366
for (const m of newMessages) state.pushedMessageIds.add(m.info?.id || m.id);
335367
state.lineOffset += lines.length;
368+
if (isFinal) {
369+
cleanupSession(sessionId);
370+
agentSessions.delete(sessionId);
371+
}
336372
} catch { /* Non-blocking */ }
337373
}
338374
},
@@ -740,22 +776,27 @@ def _collect_opencode_hook_plugins(hook_configs: list[dict]) -> list[dict]:
740776

741777
def _opencode_command_hook_plugin(name: str, event: str, command: str) -> str:
742778
"""Generate a plugin file that runs a shell command on an OpenCode event."""
743-
cmd_escaped = command.replace("\\", "\\\\").replace('"', '\\"').replace("`", "\\`")
779+
import json as _json
780+
781+
cmd_json = _json.dumps(command)
744782
return f"""// Observal hook plugin: {name}
745783
// Event: {event}
746784
// Auto-generated by `observal pull`
747785
748786
import {{ execSync }} from "child_process";
749787
788+
const HOOK_COMMAND = {cmd_json};
789+
750790
export const Hook_{name.replace("-", "_")} = async (ctx) => {{
751791
return {{
752792
event: async ({{ event }}) => {{
753793
if (event?.type === "{event}") {{
754794
try {{
755-
execSync("{cmd_escaped}", {{
795+
execSync(HOOK_COMMAND, {{
756796
cwd: ctx.directory,
757797
timeout: 10000,
758798
stdio: ["pipe", "pipe", "pipe"],
799+
shell: true,
759800
}});
760801
}} catch {{
761802
// Non-blocking: don't break the session
@@ -769,25 +810,39 @@ def _opencode_command_hook_plugin(name: str, event: str, command: str) -> str:
769810

770811
def _opencode_http_hook_plugin(name: str, event: str, url: str, timeout: int = 10) -> str:
771812
"""Generate a plugin file that makes an HTTP request on an OpenCode event."""
772-
url_escaped = url.replace("\\", "\\\\").replace('"', '\\"')
813+
from urllib.parse import urlparse
814+
815+
parsed = urlparse(url)
816+
if parsed.scheme not in ("http", "https") or not parsed.netloc:
817+
return f"// Observal hook plugin: {name} — SKIPPED (invalid URL)\nexport {{}};\n"
818+
import json as _json
819+
820+
url_json = _json.dumps(url)
821+
is_https = url.startswith("https")
822+
req_module = "https" if is_https else "http"
773823
return f"""// Observal hook plugin: {name}
774824
// Event: {event}
775825
// Auto-generated by `observal pull`
776826
777-
import {{ execFileSync }} from "child_process";
827+
import {{ request }} from "{req_module}";
828+
829+
const HOOK_URL = {url_json};
778830
779831
export const Hook_{name.replace("-", "_")} = async (ctx) => {{
780832
return {{
781833
event: async ({{ event }}) => {{
782834
if (event?.type === "{event}") {{
783835
try {{
784-
execFileSync("curl", [
785-
"-s", "-X", "POST",
786-
"-H", "Content-Type: application/json",
787-
"--max-time", "{timeout}",
788-
"-d", JSON.stringify({{ event: event?.type, properties: event?.properties || {{}} }}),
789-
"{url_escaped}",
790-
], {{ timeout: {timeout * 1000 + 2000}, stdio: ["pipe", "pipe", "pipe"] }});
836+
const body = JSON.stringify({{ event: event?.type, properties: event?.properties || {{}} }});
837+
const req = request(HOOK_URL, {{
838+
method: "POST",
839+
headers: {{ "Content-Type": "application/json", "Content-Length": Buffer.byteLength(body) }},
840+
timeout: {timeout * 1000},
841+
}});
842+
req.on("error", () => {{}});
843+
req.on("timeout", () => {{ req.destroy(); }});
844+
req.write(body);
845+
req.end();
791846
}} catch {{
792847
// Non-blocking
793848
}}

observal_cli/ide/opencode.py

Lines changed: 45 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from __future__ import annotations
1111

1212
import json
13+
import re
1314
from pathlib import Path
1415
from typing import Any
1516

@@ -24,6 +25,20 @@
2425
)
2526
from observal_cli.ide.base import BaseAdapter
2627

28+
_JSONC_COMMENT_RE = re.compile(
29+
r'("(?:[^"\\]|\\.)*")|//[^\n]*|/\*.*?\*/',
30+
re.DOTALL,
31+
)
32+
33+
34+
def _strip_jsonc_comments(text: str) -> str:
35+
"""Strip // and /* */ comments from JSONC, preserving strings."""
36+
def _replacer(m: re.Match) -> str:
37+
if m.group(1) is not None:
38+
return m.group(1)
39+
return ""
40+
return _JSONC_COMMENT_RE.sub(_replacer, text)
41+
2742

2843
class OpenCodeAdapter(BaseAdapter):
2944
"""Adapter for OpenCode."""
@@ -84,31 +99,26 @@ def generate_hook_config(
8499
def detect_hooks(self, config_dir: Path) -> str:
85100
"""Detect if the Observal plugin is installed in OpenCode.
86101
87-
Checks both project-level and global plugin directories.
102+
Checks the global plugin directory (config_dir/plugins) which is
103+
the canonical install location for `observal pull` with user scope.
88104
"""
89-
# Check project-level plugins
90-
project_plugin = Path.cwd() / ".opencode" / "plugins"
91-
global_plugin = config_dir / "plugins"
105+
plugins_dir = config_dir / "plugins"
106+
if not plugins_dir.exists():
107+
return "missing"
92108

93-
found = 0
94-
for plugins_dir in (project_plugin, global_plugin):
95-
if not plugins_dir.exists():
109+
for plugin_file in plugins_dir.iterdir():
110+
if not plugin_file.is_file():
111+
continue
112+
if plugin_file.suffix not in (".ts", ".js", ".mjs"):
113+
continue
114+
try:
115+
content = plugin_file.read_text(errors="ignore")
116+
if "ObservalPlugin" in content or "observal" in content.lower():
117+
return "installed"
118+
except OSError:
96119
continue
97-
for plugin_file in plugins_dir.iterdir():
98-
if not plugin_file.is_file():
99-
continue
100-
if plugin_file.suffix not in (".ts", ".js", ".mjs"):
101-
continue
102-
try:
103-
content = plugin_file.read_text(errors="ignore")
104-
if "ObservalPlugin" in content or "observal" in content.lower():
105-
found += 1
106-
except OSError:
107-
continue
108-
109-
if found == 0:
110-
return "missing"
111-
return "installed"
120+
121+
return "missing"
112122

113123
def shim_status(self, mcps: list[DiscoveredMcp]) -> str:
114124
return super().shim_status(mcps)
@@ -170,14 +180,7 @@ def _parse_opencode_config(self, config_file: Path, source: str) -> ScanResult:
170180
"""Parse an opencode.json config and extract MCP servers."""
171181
try:
172182
raw = config_file.read_text()
173-
# Strip JSONC comments (single-line // comments)
174-
lines = []
175-
for line in raw.splitlines():
176-
stripped = line.lstrip()
177-
if stripped.startswith("//"):
178-
continue
179-
lines.append(line)
180-
data = json.loads("\n".join(lines))
183+
data = json.loads(_strip_jsonc_comments(raw))
181184
servers = data.get("mcp", {})
182185
mcps = []
183186
for srv_name, srv_config in servers.items():
@@ -281,21 +284,25 @@ def _scan_plugins_dir(self, plugins_dir: Path, source: str) -> list[DiscoveredHo
281284
return hooks
282285

283286
def _extract_frontmatter_field(self, content: str, field: str) -> str | None:
284-
"""Extract a field from YAML frontmatter in a markdown file."""
287+
"""Extract a top-level field from YAML frontmatter in a markdown file."""
285288
if not content.startswith("---"):
286289
return None
287290
parts = content.split("---", 2)
288291
if len(parts) < 3:
289292
return None
290293
frontmatter = parts[1]
294+
prefix = f"{field}:"
291295
for line in frontmatter.splitlines():
292-
line = line.strip()
293-
if line.startswith(f"{field}:"):
294-
value = line[len(f"{field}:") :].strip()
295-
# Remove surrounding quotes if present
296-
if value and value[0] in ('"', "'") and value[-1] == value[0]:
297-
value = value[1:-1]
298-
return value
296+
# Only match top-level keys (no leading whitespace)
297+
if line.startswith((" ", "\t")):
298+
continue
299+
stripped = line.strip()
300+
if not stripped.startswith(prefix):
301+
continue
302+
value = stripped[len(prefix):].strip()
303+
if value and value[0] in ('"', "'") and value[-1] == value[0]:
304+
value = value[1:-1]
305+
return value
299306
return None
300307

301308

0 commit comments

Comments
 (0)