From bdf7cb886ee5a4c5708a704324a102a5de513fe5 Mon Sep 17 00:00:00 2001 From: SkyFi Geek <45924209+mobileskyfi@users.noreply.github.com> Date: Tue, 16 Jun 2026 09:29:56 -0700 Subject: [PATCH 1/2] test(protocols): pin native-api value escaping round-trip + ssh no-PTY (JG-09/JG-15) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Promote the protocol-compliance facts the fan-out flagged as untested into GitHub-visible assertions, independent of any command/feature: - JG-15: native-api attribute *value* round-trip — the API protocol is length-prefix framed, not delimiter-escaped, so `=`, spaces, CR/LF, NUL, and multibyte UTF-8 survive verbatim, and parseReply splits `=name=value` on the second `=` so a value containing `=` is read back whole. Pins the full attributeWord → encodeSentence → SentenceReader → parseReply path; the codec was tested for framing but never for special-char values end-to-end. - execute/ssh no-PTY guard: SshExecClient.argv must never request a tty (`-t`, `-tt`, `-T`, RequestTTY) — RouterOS grants no PTY and `-tt` hangs it. The terminal relay already guards this (terminal.test.ts); execute did not. - attributeWord doc comment records the no-escaping contract the test pins. Test-only + doc comment; native-api codec unchanged; cells stay CHR-passed. Co-Authored-By: Claude Opus 4.8 --- src/protocols/native-api.ts | 10 ++++++- test/unit/native-api.test.ts | 53 ++++++++++++++++++++++++++++++++++++ test/unit/ssh.test.ts | 11 ++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/protocols/native-api.ts b/src/protocols/native-api.ts index ecf0c4d..3c16928 100644 --- a/src/protocols/native-api.ts +++ b/src/protocols/native-api.ts @@ -307,7 +307,15 @@ export function challengeResponse( return `00${hash.digest("hex")}`; } -/** Build an API command word `=name=value` attribute word. */ +/** + * Build an API command word `=name=value` attribute word. + * + * The value needs **no escaping**: words are length-prefix framed on the wire + * ({@link encodeWord}), so any bytes — `=`, spaces, CR/LF, NUL, multibyte UTF-8 — + * pass through verbatim, and {@link parseReply} splits on the *second* `=` so a + * value that contains `=` is read back whole. Round-trip pinned in + * `test/unit/native-api.test.ts` ("attribute value round-trip (JG-15)"). + */ export function attributeWord(name: string, value: string): string { return `=${name}=${value}`; } diff --git a/test/unit/native-api.test.ts b/test/unit/native-api.test.ts index f5ecd4b..4bb0a83 100644 --- a/test/unit/native-api.test.ts +++ b/test/unit/native-api.test.ts @@ -117,6 +117,59 @@ describe("native-api word and sentence framing", () => { }); }); +/** + * JG-15 — attribute *value* round-trip across the codec, the worry case the + * other transports escape but native-api does not. The API protocol is + * length-prefix framed, not delimiter-escaped: a value is a count of raw bytes, + * so `=`, spaces, CR/LF, NUL, and multibyte UTF-8 all survive verbatim with no + * escaping. `parseReply` splits `=name=value` on the *second* `=` + * ({@link parseReply} uses `indexOf("=", 1)`), so a value that itself contains + * `=` is returned whole, not truncated. This pins that no caller-side trimming + * or naive `split("=")` corrupts a value end-to-end: + * `attributeWord → encodeSentence → SentenceReader → parseReply`. + */ +describe("native-api attribute value round-trip (JG-15)", () => { + /** Send one `=name=value` attribute through the full codec and read it back. */ + function roundTrip(name: string, value: string): string | undefined { + const words = ["!re", attributeWord(name, value)]; + const reader = new SentenceReader(); + const sentences = reader.push(encodeSentence(words)); + expect(sentences).toHaveLength(1); + return readAttribute(parseReply(sentences[0] as string[]), name); + } + + const cases: Array<[label: string, value: string]> = [ + ["a value containing '=' (split on the second =)", "key1=val1=val2"], + ["an embedded RouterOS expression with =", "([/system/clock/get date]=x)"], + ["spaces (an unquoted comment)", "office uplink — do not touch"], + ["a literal CR/LF inside the value", "line1\r\nline2"], + ["a tab and other control bytes", "a\tb\x01c"], + ["a NUL byte mid-value (framing is length-prefixed)", "before\x00after"], + ["multibyte UTF-8", "café — ☃ — 日本語 — 🛰️"], + [ + "RouterOS escapes left literal (no double-escaping)", + '\\"quoted\\" $x \\n ;', + ], + ["leading/trailing whitespace is preserved", " padded "], + ["the empty value", ""], + ["a long value spanning a multi-byte length prefix", "z".repeat(0x250)], + ]; + + for (const [label, value] of cases) { + test(label, () => { + expect(roundTrip("comment", value)).toBe(value); + }); + } + + test("a value-less attribute (`=name=`) reads back as the empty string", () => { + // parseReply maps both `=name=` and a bare `=name` to "". + expect(roundTrip("disabled", "")).toBe(""); + const reader = new SentenceReader(); + const [sentence] = reader.push(encodeSentence(["!re", "=name"])); + expect(readAttribute(parseReply(sentence as string[]), "name")).toBe(""); + }); +}); + describe("native-api SentenceReader streaming", () => { test("reassembles a sentence split across arbitrary byte boundaries", () => { const sentence = encodeSentence(["!re", "=name=ether1", "=type=ether"]); diff --git a/test/unit/ssh.test.ts b/test/unit/ssh.test.ts index e8a31cb..15e5599 100644 --- a/test/unit/ssh.test.ts +++ b/test/unit/ssh.test.ts @@ -77,6 +77,17 @@ describe("SshExecClient", () => { expect(argv.at(-1)).toBe("/system/identity/print"); }); + test("never forces a PTY — RouterOS grants none and `-tt` hangs it", () => { + // Per-command batch exec must not request a tty (the terminal relay guards + // the same in terminal.test.ts). `-tt` makes the host ssh demand a PTY that + // RouterOS refuses, hanging the session (CHR 7.23.1 grounded). + const argv = client({ exitCode: 0, stdout: "", stderr: "" }).argv("/x"); + expect(argv).not.toContain("-t"); + expect(argv).not.toContain("-tt"); + expect(argv).not.toContain("-T"); + expect(argv.join(" ")).not.toContain("RequestTTY"); + }); + test("exec returns cleaned stdout on success", async () => { const out = await client({ exitCode: 0, From 9a49c0abda6ea1bef0521420f4820f401993db42 Mon Sep 17 00:00:00 2001 From: SkyFi Geek <45924209+mobileskyfi@users.noreply.github.com> Date: Tue, 16 Jun 2026 09:55:21 -0700 Subject: [PATCH 2/2] =?UTF-8?q?test(protocols):=20address=20Copilot=20#48?= =?UTF-8?q?=20=E2=80=94=20forbid=20only=20PTY-forcing,=20drop=20casts,=20U?= =?UTF-8?q?TF-8=20wording?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ssh no-PTY guard: only forbid *forcing* a PTY (`-t`/`-tt`/`RequestTTY=force|yes`); `-T` and `RequestTTY=no` *disable* a PTY and are valid hardening, so allow them. - native-api round-trip: destructure + guard instead of `as string[]` casts, so a decode regression to `undefined` fails loudly rather than being masked. - attributeWord doc + JG-15 test comment: "any UTF-8 content" (encodeWord UTF-8 encodes a JS string by byte length), not "any bytes" — avoids over-promising raw-binary safety while still covering `=`, spaces, CR/LF, NUL, multibyte. Co-Authored-By: Claude Opus 4.8 --- src/protocols/native-api.ts | 9 +++++---- test/unit/native-api.test.ts | 16 +++++++++------- test/unit/ssh.test.ts | 9 +++++---- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/protocols/native-api.ts b/src/protocols/native-api.ts index 3c16928..e834274 100644 --- a/src/protocols/native-api.ts +++ b/src/protocols/native-api.ts @@ -311,10 +311,11 @@ export function challengeResponse( * Build an API command word `=name=value` attribute word. * * The value needs **no escaping**: words are length-prefix framed on the wire - * ({@link encodeWord}), so any bytes — `=`, spaces, CR/LF, NUL, multibyte UTF-8 — - * pass through verbatim, and {@link parseReply} splits on the *second* `=` so a - * value that contains `=` is read back whole. Round-trip pinned in - * `test/unit/native-api.test.ts` ("attribute value round-trip (JG-15)"). + * ({@link encodeWord} UTF-8-encodes by byte length), so any UTF-8 content — `=`, + * spaces, CR/LF, NUL, multibyte characters — passes through verbatim, and + * {@link parseReply} splits on the *second* `=` so a value that contains `=` is + * read back whole. Round-trip pinned in `test/unit/native-api.test.ts` + * ("attribute value round-trip (JG-15)"). */ export function attributeWord(name: string, value: string): string { return `=${name}=${value}`; diff --git a/test/unit/native-api.test.ts b/test/unit/native-api.test.ts index 4bb0a83..6c62cbe 100644 --- a/test/unit/native-api.test.ts +++ b/test/unit/native-api.test.ts @@ -120,9 +120,9 @@ describe("native-api word and sentence framing", () => { /** * JG-15 — attribute *value* round-trip across the codec, the worry case the * other transports escape but native-api does not. The API protocol is - * length-prefix framed, not delimiter-escaped: a value is a count of raw bytes, - * so `=`, spaces, CR/LF, NUL, and multibyte UTF-8 all survive verbatim with no - * escaping. `parseReply` splits `=name=value` on the *second* `=` + * length-prefix framed, not delimiter-escaped: a word is a UTF-8 byte count, so + * any UTF-8 content — `=`, spaces, CR/LF, NUL, multibyte characters — survives + * verbatim with no escaping. `parseReply` splits `=name=value` on the *second* `=` * ({@link parseReply} uses `indexOf("=", 1)`), so a value that itself contains * `=` is returned whole, not truncated. This pins that no caller-side trimming * or naive `split("=")` corrupts a value end-to-end: @@ -133,9 +133,10 @@ describe("native-api attribute value round-trip (JG-15)", () => { function roundTrip(name: string, value: string): string | undefined { const words = ["!re", attributeWord(name, value)]; const reader = new SentenceReader(); - const sentences = reader.push(encodeSentence(words)); - expect(sentences).toHaveLength(1); - return readAttribute(parseReply(sentences[0] as string[]), name); + const [sentence, ...rest] = reader.push(encodeSentence(words)); + expect(rest).toHaveLength(0); + if (!sentence) throw new Error("codec decoded no sentence"); + return readAttribute(parseReply(sentence), name); } const cases: Array<[label: string, value: string]> = [ @@ -166,7 +167,8 @@ describe("native-api attribute value round-trip (JG-15)", () => { expect(roundTrip("disabled", "")).toBe(""); const reader = new SentenceReader(); const [sentence] = reader.push(encodeSentence(["!re", "=name"])); - expect(readAttribute(parseReply(sentence as string[]), "name")).toBe(""); + if (!sentence) throw new Error("codec decoded no sentence"); + expect(readAttribute(parseReply(sentence), "name")).toBe(""); }); }); diff --git a/test/unit/ssh.test.ts b/test/unit/ssh.test.ts index 15e5599..e6dbd4f 100644 --- a/test/unit/ssh.test.ts +++ b/test/unit/ssh.test.ts @@ -78,14 +78,15 @@ describe("SshExecClient", () => { }); test("never forces a PTY — RouterOS grants none and `-tt` hangs it", () => { - // Per-command batch exec must not request a tty (the terminal relay guards + // Per-command batch exec must not *force* a tty (the terminal relay guards // the same in terminal.test.ts). `-tt` makes the host ssh demand a PTY that - // RouterOS refuses, hanging the session (CHR 7.23.1 grounded). + // RouterOS refuses, hanging the session (CHR 7.23.1 grounded). Disabling a + // PTY (`-T`, `RequestTTY=no`) is the opposite and would be fine, so only the + // forcing forms are forbidden. const argv = client({ exitCode: 0, stdout: "", stderr: "" }).argv("/x"); expect(argv).not.toContain("-t"); expect(argv).not.toContain("-tt"); - expect(argv).not.toContain("-T"); - expect(argv.join(" ")).not.toContain("RequestTTY"); + expect(argv.join(" ")).not.toMatch(/RequestTTY[= ](?:force|yes)/i); }); test("exec returns cleaned stdout on success", async () => {