A kept command's first word is parted by any whitespace, and capped (review of issue 397)
The review of #210 found that the fix left the hole open and widened another. A command's words were cut on a space alone, while the verb's own splitter parts them on a space, a tab or a newline. The same line written with tabs found no space, so all of it was kept — the very hole this closes. Its words are now parted as the splitter parts them. And because the command arm sits above the arm that caps a string at 120 bytes, a command kept whole was no longer capped: for a 300-byte single token the change kept more than the code it replaced. The first word now carries the same cap. A one-word command is still kept whole, but the comment no longer claims it carries nothing to withhold: the record is written before the verb judges the line, so one word may be a token or compact JSON. The cap is what bounds that. The test now says the whole of what is kept for each line, which is also what proves the rest is gone, and looks for forbidden words as whole words rather than as substrings — so "set" is back in the list, and "show" cannot hide in a word such as "shown". All eight cases fail against the unfixed code.
This commit is contained in:
+18
-7
@@ -469,19 +469,30 @@ func kept(args json.RawMessage) json.RawMessage {
|
||||
out[k] = []any{words[0], "(the rest given, not kept)"}
|
||||
}
|
||||
case k == "command":
|
||||
// The controller's own command line, kept as `line` is: its first word, never the rest.
|
||||
// `settings set <module> '<value>' --node <node>` through the `command` verb put the value
|
||||
// itself in this record, which answers anyone who may call the seat (novox/hq issue 397).
|
||||
// The controller's own command line: its first word, never the rest, as a mesh-cli line's
|
||||
// first word is. `settings set <module> '<value>' --node <node>` through the `command` verb
|
||||
// put the value itself in this record, which answers anyone who may call the seat (novox/hq
|
||||
// issue 397). Its words are parted by any whitespace, because the verb's own splitter parts
|
||||
// them on a space, a tab or a newline, and a line written with tabs is the same line. The
|
||||
// first word is capped as every other string here is: the record is written before the verb
|
||||
// judges the line, so one word may be anything a caller sent, a token included.
|
||||
if !isString {
|
||||
out[k] = "(given, not kept)"
|
||||
break
|
||||
}
|
||||
first, rest, _ := strings.Cut(strings.TrimSpace(s), " ")
|
||||
if strings.TrimSpace(rest) == "" {
|
||||
out[k] = first
|
||||
words := strings.Fields(s)
|
||||
if len(words) == 0 {
|
||||
out[k] = ""
|
||||
break
|
||||
}
|
||||
out[k] = first + " (the rest given, not kept)"
|
||||
first := words[0]
|
||||
if len(first) > 120 {
|
||||
first = first[:120] + "…"
|
||||
}
|
||||
if len(words) > 1 {
|
||||
first += " (the rest given, not kept)"
|
||||
}
|
||||
out[k] = first
|
||||
case isString && len(s) <= 120:
|
||||
out[k] = s
|
||||
case isString:
|
||||
|
||||
+34
-21
@@ -11,6 +11,7 @@ import (
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
"unicode"
|
||||
|
||||
"github.com/nats-io/nats.go"
|
||||
|
||||
@@ -167,37 +168,49 @@ func TestACallKeepsNoSettingsOrSecrets(t *testing.T) {
|
||||
}
|
||||
|
||||
// A command line's own words may carry a setting's value or a secret, so only its first word is kept
|
||||
// — as a mesh-cli line's is (novox/hq issue 397).
|
||||
// — as a mesh-cli line's is (novox/hq issue 397). Each case says the whole of what is kept, because
|
||||
// what matters is as much what is left out as what is there.
|
||||
func TestACallKeepsNoCommandLine(t *testing.T) {
|
||||
for _, line := range []string{
|
||||
`settings set notes 'the operator's own passphrase' --node laptop`,
|
||||
`settings set notes --values {"token":"s3cret"}`,
|
||||
` node show laptop`,
|
||||
long := strings.Repeat("s3cret", 50) // one word, 300 bytes: a token, as far as this can tell
|
||||
for line, want := range map[string]string{
|
||||
`settings set notes 'the operator's own passphrase' --node laptop`: "settings (the rest given, not kept)",
|
||||
`settings set notes --values {"token":"s3cret"}`: "settings (the rest given, not kept)",
|
||||
// Parted by a tab or a newline, which the verb's splitter reads as this line's words too.
|
||||
"settings\tset\tnotes\tthe-operators-passphrase": "settings (the rest given, not kept)",
|
||||
"builds\n--limit 5": "builds (the rest given, not kept)",
|
||||
"builds\t--limit 5": "builds (the rest given, not kept)",
|
||||
// Its first word is kept, and the spaces before it are not part of it.
|
||||
` node show laptop`: "node (the rest given, not kept)",
|
||||
// A command of one word is kept whole: there is nothing after it to withhold.
|
||||
`builds`: "builds",
|
||||
// One word is still capped, as every other string in a kept record is.
|
||||
long: long[:120] + "…",
|
||||
} {
|
||||
raw, err := json.Marshal(map[string]any{"command": line})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
got := string(kept(raw))
|
||||
// "set" is left out: it is a part of "settings", the word that is kept.
|
||||
for _, never := range []string{"passphrase", "s3cret", "notes", "laptop", "show"} {
|
||||
if strings.Contains(got, never) {
|
||||
t.Errorf("kept %s of %q: it carries %q", got, line, never)
|
||||
var got struct{ Command string }
|
||||
if err := json.Unmarshal(kept(raw), &got); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got.Command != want {
|
||||
t.Errorf("%q is kept as %q; want %q", line, got.Command, want)
|
||||
}
|
||||
// Whole words, so that "set" can be looked for although "settings" is kept, and "show" cannot
|
||||
// be found in a word such as "shown".
|
||||
for _, never := range []string{"set", "notes", "laptop", "show", "passphrase", "operators"} {
|
||||
for _, word := range strings.FieldsFunc(got.Command, func(r rune) bool { return !unicode.IsLetter(r) }) {
|
||||
if word == never {
|
||||
t.Errorf("%q is kept as %q: it carries the word %q", line, got.Command, never)
|
||||
}
|
||||
}
|
||||
}
|
||||
if !strings.Contains(got, "(the rest given, not kept)") {
|
||||
t.Errorf("kept %s of %q: it does not say the rest was given", got, line)
|
||||
}
|
||||
}
|
||||
|
||||
// A command of one word is the whole of it: a reading command with no arguments stays readable.
|
||||
raw, _ := json.Marshal(map[string]any{"command": "builds"})
|
||||
if got := string(kept(raw)); got != `{"command":"builds"}` {
|
||||
t.Errorf("kept %s; a command of one word carries no value to withhold", got)
|
||||
}
|
||||
|
||||
// Not a string, so its first word cannot be taken: none of it is kept.
|
||||
raw, _ = json.Marshal(map[string]any{"command": []string{"settings", "set", "notes", "s3cret"}})
|
||||
// Not a string, so its first word cannot be taken: none of it is kept. The verb refuses such a
|
||||
// call, but the record is written before it judges it.
|
||||
raw, _ := json.Marshal(map[string]any{"command": []string{"settings", "set", "notes", "s3cret"}})
|
||||
if got := string(kept(raw)); strings.Contains(got, "s3cret") {
|
||||
t.Errorf("kept %s", got)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user