From 36525ba82fe97cb69927f5c00a44370d834e98c1 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Wed, 2 Sep 2026 09:15:17 +0800 Subject: [PATCH 01/14] fix(zapscript): reject ZapScript over a maximum length Nothing bounded the length of ZapScript text anywhere. RunParams.Text carried no validate tag, mapping Override was uncapped while Label was limited to 255, History.TokenValue is unconstrained text, and the MQTT, file, serial, barcode, optical, external drive and GMC readers passed their payload straight through. The only ceilings were the 4MB WebSocket and 1MB HTTP POST envelopes. Parse cost grows with the length of the text, so unbounded input is a way to occupy the single token worker that every reader queues through, and, once stored, to make every later history read expensive for the whole retention window. MaxScriptLength is 8192 bytes, well above an NTAG216's 888 and any hand-written script. It is applied at the four points untrusted text arrives: the JSON-RPC run method for both param shapes, the REST handler before IsRunAllowed parses the URL, a ZapLink response body, and handleQueuedToken as the backstop covering every reader at the one place they converge. An over-long token completes with an error and is not written to history, matching the empty-token case beside it. Refs #1375 --- pkg/api/methods/methods_test.go | 33 ++++++++++++ pkg/api/methods/run.go | 19 +++++++ pkg/api/methods/run_completion_test.go | 39 +++++++++++++++ pkg/api/models/params.go | 4 +- pkg/service/queues.go | 21 ++++++++ pkg/service/token_completion_test.go | 44 ++++++++++++++++ pkg/zapscript/commands.go | 5 ++ pkg/zapscript/limits.go | 48 ++++++++++++++++++ pkg/zapscript/limits_test.go | 69 ++++++++++++++++++++++++++ 9 files changed, 280 insertions(+), 2 deletions(-) create mode 100644 pkg/zapscript/limits.go create mode 100644 pkg/zapscript/limits_test.go diff --git a/pkg/api/methods/methods_test.go b/pkg/api/methods/methods_test.go index 91a4ee6b7..2b0a8ab8d 100644 --- a/pkg/api/methods/methods_test.go +++ b/pkg/api/methods/methods_test.go @@ -25,6 +25,7 @@ import ( "errors" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -40,6 +41,7 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/tokens" "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/helpers" "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/mocks" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/zapscript" "github.com/go-chi/chi/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -184,6 +186,37 @@ func TestHandleRunRestRejectsMalformedEscapedPath(t *testing.T) { } } +// The REST path hands its text to IsRunAllowed, which parses it, so the +// length bound has to apply before that and before the token is queued. +func TestHandleRunRestRejectsOversizedScript(t *testing.T) { + t.Parallel() + + platform := mocks.NewMockPlatform() + platform.SetupBasicMock() + st, _ := state.NewState(platform, "test-boot-uuid") + t.Cleanup(st.StopService) + + tokenQueue := make(chan tokens.Token, 1) + router := chi.NewRouter() + router.Get("/run/*", HandleRunRest(&config.Instance{}, st, tokenQueue)) + + oversized := strings.Repeat("A", zapscript.MaxScriptLength+1) + req := httptest.NewRequestWithContext( + context.Background(), http.MethodGet, "/run/"+oversized, http.NoBody, + ) + req.RemoteAddr = "127.0.0.1:1234" + recorder := httptest.NewRecorder() + + router.ServeHTTP(recorder, req) + + assert.Equal(t, http.StatusRequestEntityTooLarge, recorder.Code) + select { + case token := <-tokenQueue: + t.Fatalf("REST run handler queued an over-long token: %d bytes", len(token.Text)) + default: + } +} + func TestHandleRunReturnsWhenRequestContextCancelled(t *testing.T) { t.Parallel() diff --git a/pkg/api/methods/run.go b/pkg/api/methods/run.go index 6afea0a7a..f2420f91c 100644 --- a/pkg/api/methods/run.go +++ b/pkg/api/methods/run.go @@ -86,6 +86,14 @@ func HandleRun(env requests.RequestEnv) (any, error) { //nolint:gocritic // sing return nil, models.ClientErrf("invalid params: %w", err) } + // Bound the text before anything parses it, including the redaction + // below. + if params.Text != nil { + if lenErr := zapscript.ValidateScriptLength(*params.Text); lenErr != nil { + return nil, models.ClientErr(lenErr) + } + } + log.Debug().Msgf("unmarshalled run params: %+v", runParamsForLog(¶ms)) if params.Type != nil { @@ -129,6 +137,10 @@ func HandleRun(env requests.RequestEnv) (any, error) { //nolint:gocritic // sing return nil, models.ClientErr(validation.ErrMissingParams) } + if lenErr := zapscript.ValidateScriptLength(text); lenErr != nil { + return nil, models.ClientErr(lenErr) + } + t.Text = norm.NFC.String(text) } @@ -268,6 +280,13 @@ func HandleRunRest( } } + // IsRunAllowed parses the text, so bound it first. + if err := zapscript.ValidateScriptLength(text); err != nil { + log.Warn().Err(err).Msg("rejecting over-long REST run request") + http.Error(w, http.StatusText(http.StatusRequestEntityTooLarge), http.StatusRequestEntityTooLarge) + return + } + if !isLocalRequest(r) && !cfg.IsRunAllowed(text) { log.Warn().Msg("REST run not allowed") http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden) diff --git a/pkg/api/methods/run_completion_test.go b/pkg/api/methods/run_completion_test.go index 4fdfbb63d..74b17797f 100644 --- a/pkg/api/methods/run_completion_test.go +++ b/pkg/api/methods/run_completion_test.go @@ -23,6 +23,7 @@ import ( "context" "errors" "fmt" + "strings" "testing" "time" @@ -146,6 +147,44 @@ func TestHandleRunWaitsForCompletionThenSucceeds(t *testing.T) { assert.Equal(t, NoContent{}, o.result) } +// Over-long text is rejected before it reaches the redaction in the debug +// log or the token queue, so neither the API goroutine nor the service worker +// ever parses it. +func TestHandleRunRejectsOversizedScript(t *testing.T) { + t.Parallel() + + oversized := "**launch:" + strings.Repeat("A", zapscript.MaxScriptLength) + + t.Run("object params", func(t *testing.T) { + t.Parallel() + env := newRunTestEnv(t) + o := waitRun(t, startRun(env.requestEnv(context.Background(), oversized))) + + require.ErrorIs(t, o.err, zapscript.ErrScriptTooLong) + select { + case tok := <-env.queue: + t.Fatalf("run queued an over-long token: %d bytes", len(tok.Text)) + default: + } + }) + + t.Run("bare string params", func(t *testing.T) { + t.Parallel() + env := newRunTestEnv(t) + reqEnv := env.requestEnv(context.Background(), "") + reqEnv.Params = []byte(fmt.Sprintf("%q", oversized)) + + o := waitRun(t, startRun(reqEnv)) + + require.ErrorIs(t, o.err, zapscript.ErrScriptTooLong) + select { + case tok := <-env.queue: + t.Fatalf("run queued an over-long token: %d bytes", len(tok.Text)) + default: + } + }) +} + func TestHandleRunReportsExecutionFailureByCategory(t *testing.T) { t.Parallel() diff --git a/pkg/api/models/params.go b/pkg/api/models/params.go index e79653f07..5241a8192 100644 --- a/pkg/api/models/params.go +++ b/pkg/api/models/params.go @@ -113,7 +113,7 @@ type AddMappingParams struct { Type string `json:"type" validate:"required,oneof=id value data uid text"` Match string `json:"match" validate:"required,oneof=exact partial regex"` Pattern string `json:"pattern" validate:"required"` - Override string `json:"override"` + Override string `json:"override" validate:"max=8192"` Enabled bool `json:"enabled"` } @@ -127,7 +127,7 @@ type UpdateMappingParams struct { Type *string `json:"type" validate:"omitempty,oneof=id value data uid text"` Match *string `json:"match" validate:"omitempty,oneof=exact partial regex"` Pattern *string `json:"pattern" validate:"omitempty,min=1"` - Override *string `json:"override"` + Override *string `json:"override" validate:"omitempty,max=8192"` ID int `json:"id" validate:"gt=0"` } diff --git a/pkg/service/queues.go b/pkg/service/queues.go index 84d7b0faf..2f27a5ecb 100644 --- a/pkg/service/queues.go +++ b/pkg/service/queues.go @@ -768,6 +768,22 @@ var ( errLaunchPanicked = errors.New("token launch panicked") ) +// rejectOversizedToken drops a token whose script exceeds the length bound. +// +// This is the one point every source converges on — readers, the API, REST and +// the GMC proxy all reach the worker through the same channel — so bounding +// here covers the sources that have no validation of their own. It runs before +// the token is logged, redacted or stored, because each of those parses the +// text. Nothing is written to history: an over-long script is rejected, not +// recorded, matching the empty-token case above it. +func rejectOversizedToken(t *tokens.Token, err error) { + log.Warn().Err(err). + Str("source", t.Source). + Int("length", len(t.Text)). + Msg("rejecting token, script exceeds maximum length") + t.Completion.Complete(err) +} + func processTokenQueue( svc *ServiceContext, itq <-chan tokens.Token, @@ -804,6 +820,11 @@ func handleQueuedToken( return } + if lenErr := zapscript.ValidateScriptLength(t.Text); lenErr != nil { + rejectOversizedToken(&t, lenErr) + return + } + log.Info().Msgf("processing token: %v", tokenForLog(&t)) if err := svc.Platform.ScanHook(&t); err != nil { diff --git a/pkg/service/token_completion_test.go b/pkg/service/token_completion_test.go index 1dce7f755..c3efbb36a 100644 --- a/pkg/service/token_completion_test.go +++ b/pkg/service/token_completion_test.go @@ -20,6 +20,7 @@ package service import ( + "strings" "sync" "testing" "time" @@ -204,6 +205,49 @@ func TestTokenCompletion_EmptyTokenIsRejected(t *testing.T) { require.ErrorIs(t, assertCompletedOnce(t, c), errEmptyToken) } +// An over-long script is rejected before it is parsed, logged or stored. +// Storing it would keep costing on every later history read, and parsing it +// on the worker stalls every other token behind it. +func TestTokenCompletion_OversizedScriptIsRejected(t *testing.T) { + t.Parallel() + env := setupScanBehavior(t, "tap", 0) + + oversized := "**launch:" + strings.Repeat("A", zapscript.MaxScriptLength) + c := env.sendAPIToken(t, oversized) + + require.ErrorIs(t, assertCompletedOnce(t, c), zapscript.ErrScriptTooLong) + env.expectNoLaunch(t) + + select { + case he := <-env.historyCh: + t.Fatalf("an over-long token was recorded in history: %d bytes", len(he.TokenValue)) + case <-time.After(noEventWait): + } + + // The worker must still be free for the next token. + nextPath := env.gamePath("game1.gba") + c2 := env.sendAPIToken(t, nextPath) + assert.Equal(t, nextPath, env.waitForLaunch(t)) + require.NoError(t, assertCompletedOnce(t, c2)) +} + +// A script right on the limit is ordinary input and must still run. +func TestTokenCompletion_ScriptAtLengthLimitIsAccepted(t *testing.T) { + t.Parallel() + env := setupScanBehavior(t, "tap", 0) + + path := env.gamePath("game1.gba") + padding := zapscript.MaxScriptLength - len("**launch:"+path) + require.Positive(t, padding) + script := "**launch:" + path + strings.Repeat(" ", padding) + require.Len(t, script, zapscript.MaxScriptLength) + + c := env.sendAPIToken(t, script) + + assert.Equal(t, path, env.waitForLaunch(t)) + require.NoError(t, assertCompletedOnce(t, c)) +} + func TestTokenCompletion_PanicIsReportedAndWorkerContinues(t *testing.T) { t.Parallel() env := setupScanBehavior(t, "tap", 0) diff --git a/pkg/zapscript/commands.go b/pkg/zapscript/commands.go index d4ac6e864..fe1606925 100644 --- a/pkg/zapscript/commands.go +++ b/pkg/zapscript/commands.go @@ -491,6 +491,11 @@ func RunCommand( } } if linkValue != "" { + // The link body is fetched from a remote server, so it is bounded on + // the same terms as any other untrusted script. + if lenErr := ValidateScriptLength(linkValue); lenErr != nil { + return platforms.CmdResult{}, fmt.Errorf("zap link error: %w", lenErr) + } log.Info().Msgf("valid zap link, replacing cmd: %s", linkValue) reader := zapscript.NewParser(linkValue) script, parseErr := reader.ParseScript() diff --git a/pkg/zapscript/limits.go b/pkg/zapscript/limits.go new file mode 100644 index 000000000..4296cf66e --- /dev/null +++ b/pkg/zapscript/limits.go @@ -0,0 +1,48 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package zapscript + +import ( + "errors" + "fmt" +) + +// MaxScriptLength bounds ZapScript text accepted from any untrusted source, +// measured in bytes. +// +// Parse cost grows with the length of the text, so an unbounded script is a +// way to occupy the single token worker and, once stored, to make every later +// history read expensive. The limit sits well above anything legitimate: an +// NTAG216, the largest tag in common use, holds 888 bytes. +const MaxScriptLength = 8192 + +// ErrScriptTooLong is returned for text over MaxScriptLength. Such text is +// rejected before it is parsed or stored. +var ErrScriptTooLong = errors.New("zapscript exceeds maximum length") + +// ValidateScriptLength rejects ZapScript text longer than MaxScriptLength. +// Callers must apply it at the point untrusted text enters the system, before +// the text reaches a parser. +func ValidateScriptLength(text string) error { + if len(text) > MaxScriptLength { + return fmt.Errorf("%w: %d bytes (max %d)", ErrScriptTooLong, len(text), MaxScriptLength) + } + return nil +} diff --git a/pkg/zapscript/limits_test.go b/pkg/zapscript/limits_test.go new file mode 100644 index 000000000..bfd699394 --- /dev/null +++ b/pkg/zapscript/limits_test.go @@ -0,0 +1,69 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package zapscript + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateScriptLength(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + text string + wantErr bool + }{ + {name: "empty", text: "", wantErr: false}, + {name: "ordinary script", text: "**launch:/games/snes/mario.sfc", wantErr: false}, + { + name: "the largest tag in common use", + text: strings.Repeat("A", 888), + wantErr: false, + }, + {name: "exactly at the limit", text: strings.Repeat("A", MaxScriptLength), wantErr: false}, + {name: "one byte over", text: strings.Repeat("A", MaxScriptLength+1), wantErr: true}, + {name: "far over", text: strings.Repeat("A", 32000), wantErr: true}, + { + name: "multibyte counted as bytes, not runes", + // Each rune is 3 bytes, so this is under the rune count but + // over the byte limit. + text: strings.Repeat("あ", MaxScriptLength/2), + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := ValidateScriptLength(tt.text) + if !tt.wantErr { + require.NoError(t, err) + return + } + require.Error(t, err) + assert.ErrorIs(t, err, ErrScriptTooLong) + }) + } +} From cf9d3c9bef7ec176d7ec0cd3fbef4b12b94eaca3 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Wed, 2 Sep 2026 09:15:36 +0800 Subject: [PATCH 02/14] perf(api): redact run params only when debug logging is on runParamsForLog was passed as an argument to log.Debug().Msgf, so Go evaluated it before zerolog could apply the level check. It redacts the script, which parses it, so every run request paid for that parse whether or not debug logging was enabled. Refs #1375 --- pkg/api/methods/run.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/api/methods/run.go b/pkg/api/methods/run.go index f2420f91c..cba6a17ca 100644 --- a/pkg/api/methods/run.go +++ b/pkg/api/methods/run.go @@ -94,7 +94,9 @@ func HandleRun(env requests.RequestEnv) (any, error) { //nolint:gocritic // sing } } - log.Debug().Msgf("unmarshalled run params: %+v", runParamsForLog(¶ms)) + if e := log.Debug(); e.Enabled() { + e.Msgf("unmarshalled run params: %+v", runParamsForLog(¶ms)) + } if params.Type != nil { t.Type = *params.Type From 0b06278908db420fb59c9636612601921e82d39e Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Wed, 2 Sep 2026 09:15:36 +0800 Subject: [PATCH 03/14] perf(zapscript): redact from a single parse, and only when one is needed RedactToken parsed the same text twice unconditionally, once inside RedactScript and once inside HasSensitiveScript, and three times when a credential was present. It has six call sites: handleQueuedToken calls it twice per scanned token on the service worker, and HandleHistory calls it once per row, so a 25-row page cost 50 to 75 parses of stored text. Credentials only ever appear in the profile and playtime.extend commands. A command name reaches the parse tree verbatim, since the grammar admits only [a-zA-Z0-9.] in a name and normalization is a lowercase, so a name absent from the source cannot appear in the result. Checking for those two names is therefore proof, not a heuristic, and lets the parse be skipped entirely for every other token. The check folds ASCII case in place rather than lowercasing a copy, because real media paths carry capitals and the copy would be the common case. RedactToken now derives both the redacted text and the sensitivity of the payload from one parse, on the rare path where a parse still happens. At the 8KB ingest bound, RedactToken drops from 11.4ms and 71MB per call to 6.7us and no allocation. One behaviour change: text that names neither command and does not parse is no longer replaced wholesale by the redacted-script placeholder. It cannot carry a credential, so it stays readable in logs and history, which is what redaction is there to allow. Text that does name one of the two commands still fails closed exactly as before. Refs #1375 --- pkg/zapscript/redact.go | 110 ++++++++++++++++++++++++++--- pkg/zapscript/redact_bench_test.go | 98 +++++++++++++++++++++++++ pkg/zapscript/redact_test.go | 109 ++++++++++++++++++++++++++++ 3 files changed, 308 insertions(+), 9 deletions(-) create mode 100644 pkg/zapscript/redact_bench_test.go diff --git a/pkg/zapscript/redact.go b/pkg/zapscript/redact.go index 7e9f7ef85..3b6360381 100644 --- a/pkg/zapscript/redact.go +++ b/pkg/zapscript/redact.go @@ -76,15 +76,86 @@ func hasCredentialCommand(script *gozapscript.Script) bool { return false } +// credentialCommands are the commands whose arguments carry a bearer +// credential. Kept beside scriptCredentials because the two must be changed +// together: a command added there and missed here would never be redacted. +var credentialCommands = []string{ + gozapscript.ZapScriptCmdProfile, + gozapscript.ZapScriptCmdPlaytimeExtend, +} + +// mayCarryCredential reports whether text could possibly parse into a +// credential-bearing command. +// +// A command name reaches the parse tree verbatim: the grammar admits only +// [a-zA-Z0-9.] in a name and normalization is a lowercase, so a name absent +// from the source cannot appear in the result. A false return is therefore +// proof that no credential is present, whether or not the text parses, and +// the parse can be skipped entirely. That matters because this runs on every +// token scanned and on every history row read. +func mayCarryCredential(text string) bool { + for _, name := range credentialCommands { + if containsFoldASCII(text, name) { + return true + } + } + return false +} + +// containsFoldASCII reports whether s contains needle, comparing ASCII +// letters case-insensitively. needle must already be lowercase ASCII, which +// every command name is. +// +// strings.Contains(strings.ToLower(s), needle) would copy the whole script on +// every call, and real media paths carry capitals, so that copy would be the +// common case rather than the exception. +func containsFoldASCII(s, needle string) bool { + n := len(needle) + if n == 0 { + return true + } + if len(s) < n { + return false + } + + lower := needle[0] + upper := lower - ('a' - 'A') + for i := 0; i+n <= len(s); i++ { + if c := s[i]; c != lower && c != upper { + continue + } + if equalFoldASCII(s[i:i+n], needle) { + return true + } + } + return false +} + +// equalFoldASCII compares s against a lowercase ASCII needle of the same +// length, folding ASCII case in s. +func equalFoldASCII(s, needle string) bool { + for i := range len(needle) { + c := s[i] + if c >= 'A' && c <= 'Z' { + c += 'a' - 'A' + } + if c != needle[i] { + return false + } + } + return true +} + // HasSensitiveScript reports whether text involves a bearer credential, so // callers can also drop adjacent raw copies such as a token's data payload. // // This keys off the command rather than the value: a token whose text has // already been redacted may still have an unredacted raw payload beside it. -// Text that cannot be parsed is treated as sensitive, since an unreadable -// script cannot be shown to be free of credentials. +// Text that names a credential-bearing command but cannot be parsed is +// treated as sensitive, since an unreadable script cannot be shown to be free +// of credentials. func HasSensitiveScript(text string) bool { - if strings.TrimSpace(text) == "" { + if strings.TrimSpace(text) == "" || !mayCarryCredential(text) { return false } script, err := gozapscript.NewParser(text).ParseScript() @@ -103,10 +174,10 @@ func HasSensitiveScript(text string) bool { // being re-rendered from the parse tree, so traits, spacing and any command // this function does not know about survive untouched. // -// It fails closed. Text that cannot be parsed, or whose credentials survive -// the replacement, is replaced wholesale. +// It fails closed. Text that names a credential-bearing command but cannot be +// parsed, or whose credentials survive the replacement, is replaced wholesale. func RedactScript(text string) string { - if strings.TrimSpace(text) == "" { + if strings.TrimSpace(text) == "" || !mayCarryCredential(text) { return text } @@ -116,7 +187,13 @@ func RedactScript(text string) string { return redactedScript } - credentials := scriptCredentials(&script) + return redactParsedScript(text, &script) +} + +// redactParsedScript replaces the credentials of an already-parsed script in +// its own source text. +func redactParsedScript(text string, script *gozapscript.Script) string { + credentials := scriptCredentials(script) if len(credentials) == 0 { return text } @@ -140,9 +217,24 @@ func RedactScript(text string) string { // RedactToken returns a copy of a token safe to log, store, or return to API // clients. The raw data payload is dropped entirely for sensitive tokens: it // is an unparsed copy of the same content, so it cannot be redacted in place. +// +// Both answers come from a single parse. This is the hottest redaction entry +// point: it runs twice per scanned token on the service worker and once per +// row on every history read. func RedactToken(text, data string) (redactedText, redactedData string) { - redactedText = RedactScript(text) - if HasSensitiveScript(text) { + if strings.TrimSpace(text) == "" || !mayCarryCredential(text) { + return text, data + } + + script, err := gozapscript.NewParser(text).ParseScript() + if err != nil { + // Neither the script nor the raw payload beside it can be shown to + // be free of credentials. + return redactedScript, "" + } + + redactedText = redactParsedScript(text, &script) + if hasCredentialCommand(&script) { return redactedText, "" } return redactedText, data diff --git a/pkg/zapscript/redact_bench_test.go b/pkg/zapscript/redact_bench_test.go new file mode 100644 index 000000000..6257ab482 --- /dev/null +++ b/pkg/zapscript/redact_bench_test.go @@ -0,0 +1,98 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package zapscript + +import ( + "fmt" + "strings" + "testing" +) + +// benchScriptLengths spans an ordinary tag up to the ingest bound. Redaction +// runs twice per scanned token on the service worker and once per row on +// every history read, so its cost has to stay proportional to the text rather +// than to a parse of it. +var benchScriptLengths = []int{64, 512, 4096, MaxScriptLength} + +// BenchmarkRedactToken_NoCredential covers the common path: a token that +// names no credential-bearing command never reaches the parser. +func BenchmarkRedactToken_NoCredential(b *testing.B) { + for _, size := range benchScriptLengths { + script := "**launch:/games/snes/" + strings.Repeat("a", size-len("**launch:/games/snes/")) + b.Run(fmt.Sprintf("%dB", size), func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + text, data := RedactToken(script, "deadbeef") + _, _ = text, data + } + }) + } +} + +// BenchmarkRedactToken_MixedCase uses a realistic media path. Real paths +// carry capitals, which is the case a case-folding pre-check has to handle +// without copying the whole script. +func BenchmarkRedactToken_MixedCase(b *testing.B) { + for _, size := range benchScriptLengths { + prefix := "**launch:/media/fat/games/SNES/Super Metroid " + script := prefix + strings.Repeat("Ab", (size-len(prefix))/2) + b.Run(fmt.Sprintf("%dB", size), func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + text, data := RedactToken(script, "deadbeef") + _, _ = text, data + } + }) + } +} + +// BenchmarkRedactToken_ProfileCard covers the path that still parses, so a +// regression there stays visible. +func BenchmarkRedactToken_ProfileCard(b *testing.B) { + script := "**profile:sw-7f3a9c21" + b.ReportAllocs() + for b.Loop() { + text, data := RedactToken(script, "deadbeef") + _, _ = text, data + } +} + +// BenchmarkRedactToken_HistoryPage is the read-path regression guard. A page +// of history is 25 rows and every row is redacted on the way out. +func BenchmarkRedactToken_HistoryPage(b *testing.B) { + const pageSize = 25 + + for _, size := range benchScriptLengths { + rows := make([]string, pageSize) + for i := range rows { + rows[i] = fmt.Sprintf("**launch:/games/snes/%d", i) + + strings.Repeat("a", size-len("**launch:/games/snes/0")) + } + b.Run(fmt.Sprintf("%dB", size), func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + for _, row := range rows { + text, data := RedactToken(row, "deadbeef") + _, _ = text, data + } + } + }) + } +} diff --git a/pkg/zapscript/redact_test.go b/pkg/zapscript/redact_test.go index 6664a82c8..89344f85b 100644 --- a/pkg/zapscript/redact_test.go +++ b/pkg/zapscript/redact_test.go @@ -186,3 +186,112 @@ func TestRedactScript_OutputStaysParseable(t *testing.T) { }) } } + +// The pre-check is what keeps redaction off the parser for ordinary tokens, +// so it has to recognise every spelling that can reach a credential. +func TestMayCarryCredential(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + text string + want bool + }{ + {name: "profile card", text: "**profile:" + testSwitchID, want: true}, + {name: "uppercase profile card", text: "**PROFILE:" + testSwitchID, want: true}, + {name: "mixed case profile card", text: "**PrOfIlE:" + testSwitchID, want: true}, + {name: "profile clear", text: "**profile.clear", want: true}, + { + name: "extension card", + text: "**playtime.extend:15m?profile=" + testSwitchID, + want: true, + }, + { + name: "uppercase extension card", + text: "**PLAYTIME.EXTEND:15m?PROFILE=" + testSwitchID, + want: true, + }, + { + name: "credential in a chain", + text: "**launch:/games/snes/mario.sfc||**profile:" + testSwitchID, + want: true, + }, + {name: "plain launch", text: "**launch:/games/snes/mario.sfc", want: false}, + {name: "media title", text: "@SNES/Super Mario World", want: false}, + {name: "plain text", text: "just some text", want: false}, + {name: "empty", text: "", want: false}, + {name: "already redacted script", text: redactedScript, want: false}, + { + name: "playtime command that carries no credential", + text: "**playtime.pause", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, mayCarryCredential(tt.text)) + }) + } +} + +// Every command scriptCredentials can pull a credential out of must be named +// in credentialCommands, or the pre-check would skip the parse that would +// have found it. +func TestCredentialCommands_CoverScriptCredentials(t *testing.T) { + t.Parallel() + + bearers := []string{ + "**profile:" + testSwitchID, + "**playtime.extend:15m?profile=" + testSwitchID, + } + + for _, text := range bearers { + t.Run(text, func(t *testing.T) { + t.Parallel() + require.True(t, mayCarryCredential(text), + "a script this carries a credential must not skip the parse") + assert.NotContains(t, RedactScript(text), testSwitchID) + }) + } +} + +// Malformed text that cannot name a credential-bearing command is left +// readable. The parse is skipped, so nothing about it can be shown to be +// sensitive, and blanking it would only lose diagnostic value. +func TestRedactScript_KeepsMalformedTextWithoutCredentialCommand(t *testing.T) { + t.Parallel() + + malformed := `**launch:"unterminated` + + assert.Equal(t, malformed, RedactScript(malformed)) + assert.False(t, HasSensitiveScript(malformed)) +} + +// A long unrelated argument must not stop a credential elsewhere in the same +// script from being removed. +func TestRedactToken_RemovesCredentialAlongsideLongText(t *testing.T) { + t.Parallel() + + long := strings.Repeat("A", 4000) + text := "**launch:" + long + "||**profile:" + testSwitchID + + gotText, gotData := RedactToken(text, "deadbeef") + + assert.NotContains(t, gotText, testSwitchID) + assert.Contains(t, gotText, long, "unrelated content should survive") + assert.Empty(t, gotData, "the raw payload of a sensitive token is dropped") +} + +// RedactToken keeps the payload of a token that carries no credential. +func TestRedactToken_KeepsDataForOrdinaryToken(t *testing.T) { + t.Parallel() + + text := "**launch:/games/snes/mario.sfc" + + gotText, gotData := RedactToken(text, "deadbeef") + + assert.Equal(t, text, gotText) + assert.Equal(t, "deadbeef", gotData) +} From a90f9a2e74825e9702942581e9e2659f17d0c946 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Wed, 2 Sep 2026 09:15:36 +0800 Subject: [PATCH 04/14] test(service): stub the platform calls the scan benchmark reaches BenchmarkScanToLaunch_DirectPath panicked on unstubbed Settings and RootDirs mocks, which made task bench unusable. A direct path reaches PathIsLauncher, which calls DataDir and the platform's root directories. Unrelated to the parse work in this branch; it predates it and reproduces on the previous go-zapscript release. --- pkg/service/scan_to_launch_bench_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/service/scan_to_launch_bench_test.go b/pkg/service/scan_to_launch_bench_test.go index 8bddc15b7..1ce9bd88b 100644 --- a/pkg/service/scan_to_launch_bench_test.go +++ b/pkg/service/scan_to_launch_bench_test.go @@ -90,12 +90,15 @@ func setupPipelineBench(b *testing.B, n int) *pipelineBenchEnv { b.Fatal(err) } - // Mock platform — only LaunchMedia, LookupMapping, ID need stubbing + // Mock platform. Settings is reached through DataDir when a direct path + // is checked against the launcher list. pl := mocks.NewMockPlatform() pl.On("LaunchMedia", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) pl.On("LookupMapping", mock.Anything).Return("", false) pl.On("ID").Return("test-platform") pl.On("Launchers", mock.Anything).Return(benchPipelineLaunchers) + pl.On("Settings").Return(platforms.Settings{}) + pl.On("RootDirs", mock.Anything).Return([]string{}) // Launcher manager and cache lm := state.NewLauncherManager() From 87f0f7d1259a6bd2a0dbff7aae0bea5d5dcb982d Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Wed, 2 Sep 2026 09:48:57 +0800 Subject: [PATCH 05/14] chore(deps): bump go-zapscript to v0.19.0 v0.19.0 makes parsing linear in input length. Argument accumulation was quadratic, which is the other half of #1375: the length bound added earlier in this branch caps the worst case, but every parse under the cap still paid the quadratic cost until now. Parse time is 7.3ns per byte, flat from 888 bytes to 1MB. At the 8192 byte ingest bound one parse drops from roughly 5.6ms to 60us. It also removes a 4KB buffer allocated per parser, which matters because a scanned token is parsed more than once. On BenchmarkScanToLaunch the effect is 11% fewer allocations and 11% less memory per scan, with wall time unchanged: those benchmarks are dominated by MediaDB queries rather than parsing. The parse win shows up on long scripts and on allocation pressure, not on typical token latency. Closes #1375 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index c93ff6be8..83f17c124 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/Microsoft/go-winio v0.6.2 github.com/ZaparooProject/go-gameid v0.2.0 github.com/ZaparooProject/go-pn532 v0.23.0 - github.com/ZaparooProject/go-zapscript v0.18.0 + github.com/ZaparooProject/go-zapscript v0.19.0 github.com/ZaparooProject/zaparoo-core/mister v0.1.0 github.com/adrg/xdg v0.5.3 github.com/andygrunwald/vdf v1.1.0 diff --git a/go.sum b/go.sum index 6c4c3359e..459c98ba7 100644 --- a/go.sum +++ b/go.sum @@ -39,8 +39,8 @@ github.com/ZaparooProject/go-gameid v0.2.0 h1:nYDxozhwsJXacdh/lwHOJofz4SlA609WKT github.com/ZaparooProject/go-gameid v0.2.0/go.mod h1:gPQg1jQ4jgkguOJeKUiIqZeucz0GsSR67UQ/JOgYUDI= github.com/ZaparooProject/go-pn532 v0.23.0 h1:iJ5taBHFXQxYhS8zncMonWEW6pOIyklLpnVx/GFS2n4= github.com/ZaparooProject/go-pn532 v0.23.0/go.mod h1:ao2ojvudUN8ZqcBAkjodRhCjm2jDf/efS0mdSC6eDHg= -github.com/ZaparooProject/go-zapscript v0.18.0 h1:zDZI8Ll+XF5y7h50Sr7Ioq10+CeEmoqJsa37jXyYOXc= -github.com/ZaparooProject/go-zapscript v0.18.0/go.mod h1:ofo4vj6lFW0eUuSyPLt0R0JjJxExhn9eitSmFBWQVoU= +github.com/ZaparooProject/go-zapscript v0.19.0 h1:M4t3dlOrfx0g8ZkLuGt7pRhNa5SqWMs5oAgucZpovQE= +github.com/ZaparooProject/go-zapscript v0.19.0/go.mod h1:ofo4vj6lFW0eUuSyPLt0R0JjJxExhn9eitSmFBWQVoU= github.com/adrg/xdg v0.5.3 h1:xRnxJXne7+oWDatRhR1JLnvuccuIeCoBu2rtuLqQB78= github.com/adrg/xdg v0.5.3/go.mod h1:nlTsY+NNiCBGCK2tpm09vRqfVzrc2fLmXGpBLF0zlTQ= github.com/andybalholm/brotli v1.2.2 h1:HzTuoo2ErYQqf5qvcJInB8uvqSVxRttzkFexPWtnceM= From 121d6e135d1b07f03ce969905b01e0f3b36abd8b Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 3 Sep 2026 07:01:56 +0800 Subject: [PATCH 06/14] fix(zapscript): redact a credential whatever case its argument key uses scriptCredentials read the profile argument of playtime.extend with an exact map lookup, but the parser stores an advanced argument key exactly as written while the command decoder matches it into the struct field case-insensitively. So `**playtime.extend:30m?PROFILE=sw-secret` is a working extension card, granted against a real profile, whose switch ID was written to the log and to history in the clear. FuzzRedactScript could not find this because its oracle calls scriptCredentials, the same function carrying the bug, so a credential it failed to see was also a credential it failed to miss. Seeds for the mixed-case spellings are added so the corpus keeps them. advArgsFold returns every value whose key folds to the name rather than the first, because a script may carry `?profile=` and `?PROFILE=` at once and both are usable. --- pkg/zapscript/redact.go | 34 ++++++++++++++++++++----- pkg/zapscript/redact_fuzz_test.go | 5 +++- pkg/zapscript/redact_test.go | 42 +++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 7 deletions(-) diff --git a/pkg/zapscript/redact.go b/pkg/zapscript/redact.go index 3b6360381..5e188fd41 100644 --- a/pkg/zapscript/redact.go +++ b/pkg/zapscript/redact.go @@ -45,24 +45,46 @@ const redactedScript = "[redacted script]" // own output. func scriptCredentials(script *gozapscript.Script) []string { var found []string + add := func(value string) { + if value != "" && value != RedactedPlaceholder { + found = append(found, value) + } + } for i := range script.Cmds { cmd := &script.Cmds[i] - var value string switch cmd.Name { case gozapscript.ZapScriptCmdProfile: if len(cmd.Args) > 0 { - value = cmd.Args[0] + add(cmd.Args[0]) } case gozapscript.ZapScriptCmdPlaytimeExtend: - value = cmd.AdvArgs.Get(gozapscript.KeyProfile) - } - if value != "" && value != RedactedPlaceholder { - found = append(found, value) + for _, value := range advArgsFold(cmd.AdvArgs, gozapscript.KeyProfile) { + add(value) + } } } return found } +// advArgsFold returns every advanced argument value whose key matches name +// with ASCII case folded. +// +// The parser stores advanced argument keys exactly as written, while the +// command decoder matches them into struct fields case-insensitively. So +// `?PROFILE=` authorizes an extension just as `?profile=` does, and an exact +// map lookup would leave that credential in the clear. Every match is +// returned because a script may carry several spellings of the key at once. +func advArgsFold(args gozapscript.AdvArgs, name gozapscript.Key) []string { + var values []string + args.Range(func(key gozapscript.Key, value string) bool { + if strings.EqualFold(string(key), string(name)) { + values = append(values, value) + } + return true + }) + return values +} + // hasCredentialCommand reports whether any command in the script is one that // carries a bearer credential, whether or not its value has already been // replaced. diff --git a/pkg/zapscript/redact_fuzz_test.go b/pkg/zapscript/redact_fuzz_test.go index ca1f27849..4280aaba0 100644 --- a/pkg/zapscript/redact_fuzz_test.go +++ b/pkg/zapscript/redact_fuzz_test.go @@ -28,7 +28,7 @@ import ( // FuzzRedactScript checks the invariant that matters for a security // boundary: whatever untrusted token text arrives, no credential survives -// redaction. Token text comes from an NFC tag, so it is entirely attacker +// redaction. RedactToken text comes from an NFC tag, so it is entirely attacker // controlled and may be malformed in ways the parser has to survive. func FuzzRedactScript(f *testing.F) { seeds := []string{ @@ -41,6 +41,9 @@ func FuzzRedactScript(f *testing.F) { "**launch:/games/snes/mario.sfc", "**profile:", "**PROFILE:sw-secret", + "**playtime.extend:15m?PROFILE=sw-secret", + "**playtime.extend:15m?Profile=sw-secret", + "**playtime.extend:15m?profile=a&PROFILE=b", "plain text", "", } diff --git a/pkg/zapscript/redact_test.go b/pkg/zapscript/redact_test.go index 89344f85b..09dac9cdc 100644 --- a/pkg/zapscript/redact_test.go +++ b/pkg/zapscript/redact_test.go @@ -295,3 +295,45 @@ func TestRedactToken_KeepsDataForOrdinaryToken(t *testing.T) { assert.Equal(t, text, gotText) assert.Equal(t, "deadbeef", gotData) } + +// An advanced argument key reaches the command decoder case-insensitively, so +// `?PROFILE=` authorizes an extension exactly as `?profile=` does. Redaction +// has to fold case the same way, or a working extension card writes its +// credential to the log and to history in the clear. +func TestRedactScript_RemovesCredentialFromMixedCaseAdvArg(t *testing.T) { + t.Parallel() + + spellings := []string{"profile", "PROFILE", "Profile", "pRoFiLe"} + + for _, key := range spellings { + t.Run(key, func(t *testing.T) { + t.Parallel() + + text := "**playtime.extend:15m?" + key + "=" + testSwitchID + + got := RedactScript(text) + assert.NotContains(t, got, testSwitchID, "the credential must not survive") + assert.Contains(t, got, "15m", "non-sensitive content should stay readable") + + gotText, gotData := RedactToken(text, "deadbeef") + assert.NotContains(t, gotText, testSwitchID) + assert.Empty(t, gotData, "the raw payload of a sensitive token is dropped") + + assert.True(t, HasSensitiveScript(text)) + }) + } +} + +// A script can spell the key more than one way at once, and every value is a +// usable credential, so every one has to go. +func TestRedactScript_RemovesEveryCaseVariantOfTheProfileArg(t *testing.T) { + t.Parallel() + + second := "sw-0000ffff" + text := "**playtime.extend:15m?profile=" + testSwitchID + "&PROFILE=" + second + + got := RedactScript(text) + + assert.NotContains(t, got, testSwitchID) + assert.NotContains(t, got, second) +} From b5b9b8d80ce80a75b12f923c1aaa18945a6a3cfe Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 3 Sep 2026 07:02:07 +0800 Subject: [PATCH 07/14] fix(api): bound a mapping override in bytes, not runes The override replaces a token's text after that text has been length checked, so it is the one script the queue's bound never sees. A `max=8192` validate tag counts runes, so an override of 8192 three-byte characters passed validation at 24576 bytes and was stored, then parsed on the token worker. The tag is dropped in favour of ValidateScriptLength at both handlers, which measures the same unit as the limit it enforces and produces the same error as every other over-long script. --- pkg/api/methods/mappings.go | 10 ++++ pkg/api/methods/mappings_test.go | 100 +++++++++++++++++++++++++++++++ pkg/api/models/params.go | 4 +- 3 files changed, 112 insertions(+), 2 deletions(-) diff --git a/pkg/api/methods/mappings.go b/pkg/api/methods/mappings.go index 93e84112a..d68324dd2 100644 --- a/pkg/api/methods/mappings.go +++ b/pkg/api/methods/mappings.go @@ -34,6 +34,7 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database/userdb" "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/zapscript" "github.com/rs/zerolog/log" ) @@ -133,6 +134,12 @@ func HandleAddMapping(env requests.RequestEnv) (any, error) { //nolint:gocritic } } + // An override replaces the token's text after the token's own length was + // checked, so it is the one script the queue's bound never sees. + if err := zapscript.ValidateScriptLength(params.Override); err != nil { + return nil, models.ClientErrf("invalid override: %w", err) + } + m := database.Mapping{ Label: params.Label, Enabled: params.Enabled, @@ -239,6 +246,9 @@ func HandleUpdateMapping(env requests.RequestEnv) (any, error) { } if params.Override != nil { + if lenErr := zapscript.ValidateScriptLength(*params.Override); lenErr != nil { + return nil, models.ClientErrf("invalid override: %w", lenErr) + } newMapping.Override = *params.Override } diff --git a/pkg/api/methods/mappings_test.go b/pkg/api/methods/mappings_test.go index 60a387eee..07326b73d 100644 --- a/pkg/api/methods/mappings_test.go +++ b/pkg/api/methods/mappings_test.go @@ -21,7 +21,9 @@ package methods import ( "context" + "encoding/json" "path/filepath" + "strings" "testing" "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" @@ -30,8 +32,10 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database/userdb" "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/helpers" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/zapscript" "github.com/spf13/afero" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -142,3 +146,99 @@ func TestHandleMappings_InvalidParams(t *testing.T) { _, err := HandleMappings(env) require.Error(t, err) } + +// A mapping override replaces a token's text after the token's own length has +// been checked, so it is the one script the queue's bound never sees. The +// limit is in bytes, and a rune-counting check would let a multi-byte override +// through at several times the cap. +func TestHandleAddMapping_RejectsOversizedOverride(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + override string + }{ + {name: "ascii", override: strings.Repeat("a", zapscript.MaxScriptLength+1)}, + {name: "multi-byte", override: strings.Repeat("\u3042", zapscript.MaxScriptLength/2)}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + mockUserDB := helpers.NewMockUserDBI() + params, err := json.Marshal(models.AddMappingParams{ + Label: "oversized", + Type: userdb.MappingTypeID, + Match: userdb.MatchTypeExact, + Pattern: "abcdef", + Override: tt.override, + Enabled: true, + }) + require.NoError(t, err) + + env := requests.RequestEnv{ + Context: context.Background(), + Database: &database.Database{UserDB: mockUserDB}, + Params: params, + } + + _, err = HandleAddMapping(env) + require.Error(t, err) + mockUserDB.AssertNotCalled(t, "AddMapping", mock.Anything) + }) + } +} + +func TestHandleUpdateMapping_RejectsOversizedOverride(t *testing.T) { + t.Parallel() + + mockUserDB := helpers.NewMockUserDBI() + mockUserDB.On("GetMapping", int64(7)).Return(dbMappingFixture(), nil) + + override := strings.Repeat("\u3042", zapscript.MaxScriptLength/2) + params, err := json.Marshal(models.UpdateMappingParams{ + ID: 7, + Override: &override, + }) + require.NoError(t, err) + + env := requests.RequestEnv{ + Context: context.Background(), + Database: &database.Database{UserDB: mockUserDB}, + Params: params, + } + + _, err = HandleUpdateMapping(env) + require.Error(t, err) + mockUserDB.AssertNotCalled(t, "UpdateMapping", mock.Anything, mock.Anything) +} + +// An override at the limit is ordinary input and must still be accepted. +func TestHandleAddMapping_AcceptsOverrideAtLimit(t *testing.T) { + t.Parallel() + + mockUserDB := helpers.NewMockUserDBI() + mockUserDB.On("AddMapping", mock.Anything).Return(nil) + + override := "**launch:" + strings.Repeat("a", zapscript.MaxScriptLength-len("**launch:")) + params, err := json.Marshal(models.AddMappingParams{ + Label: "at limit", + Type: userdb.MappingTypeID, + Match: userdb.MatchTypeExact, + Pattern: "abcdef", + Override: override, + Enabled: true, + }) + require.NoError(t, err) + + env := requests.RequestEnv{ + Context: context.Background(), + Database: &database.Database{UserDB: mockUserDB}, + Params: params, + } + + _, err = HandleAddMapping(env) + require.NoError(t, err) + mockUserDB.AssertCalled(t, "AddMapping", mock.Anything) +} diff --git a/pkg/api/models/params.go b/pkg/api/models/params.go index 5241a8192..e79653f07 100644 --- a/pkg/api/models/params.go +++ b/pkg/api/models/params.go @@ -113,7 +113,7 @@ type AddMappingParams struct { Type string `json:"type" validate:"required,oneof=id value data uid text"` Match string `json:"match" validate:"required,oneof=exact partial regex"` Pattern string `json:"pattern" validate:"required"` - Override string `json:"override" validate:"max=8192"` + Override string `json:"override"` Enabled bool `json:"enabled"` } @@ -127,7 +127,7 @@ type UpdateMappingParams struct { Type *string `json:"type" validate:"omitempty,oneof=id value data uid text"` Match *string `json:"match" validate:"omitempty,oneof=exact partial regex"` Pattern *string `json:"pattern" validate:"omitempty,min=1"` - Override *string `json:"override" validate:"omitempty,max=8192"` + Override *string `json:"override"` ID int `json:"id" validate:"gt=0"` } From 9d3979c6266d475b4d888eaafd67cde5b650f5d5 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 3 Sep 2026 07:02:42 +0800 Subject: [PATCH 08/14] fix(service): bound a mapping override where it replaces the token text The queue bounds a token's own text, then getMapping may replace it with an override that nothing has checked. The API validates the overrides it stores, but a mappings TOML file and a platform LookupMapping never pass through the API at all, so a 9KB zapscript= line in zaparoo/mappings/*.toml reached the parser on the token worker with no bound of any kind. The check goes at the three points an override is applied rather than at load time, because pkg/config cannot import pkg/zapscript. handleQueuedToken rejects the token as it would an over-long one, so nothing is written to history; runTokenZapScript returns the error; the launch guard skips its parse and stages, which is what it already does for a script it cannot read. The mapping was also logged in full and unredacted, so an override carrying a profile switch ID leaked it on every match. --- pkg/service/queues.go | 17 ++++++++++++++++- pkg/service/readers.go | 11 +++++++++-- pkg/service/scan_behavior_test.go | 17 +++++++++++++++++ pkg/service/token_completion_test.go | 21 +++++++++++++++++++++ 4 files changed, 63 insertions(+), 3 deletions(-) diff --git a/pkg/service/queues.go b/pkg/service/queues.go index 2f27a5ecb..eff208810 100644 --- a/pkg/service/queues.go +++ b/pkg/service/queues.go @@ -146,7 +146,14 @@ func runTokenZapScriptWithContext( if len(cmds) == 0 { mappedValue, hasMapping := getMapping(svc.Config, svc.DB, svc.Platform, token) if hasMapping { - log.Info().Msgf("found mapping: %s", mappedValue) + // An override replaces the text whose length was already checked, + // and a config or platform mapping never passed through the API's + // check at all, so the substituted script is bounded here too. + if lenErr := zapscript.ValidateScriptLength(mappedValue); lenErr != nil { + return fmt.Errorf("mapping override rejected: %w", lenErr) + } + redacted, _ := zapscript.RedactToken(mappedValue, "") + log.Info().Msgf("found mapping: %s", redacted) token.Text = mappedValue } @@ -858,6 +865,14 @@ func handleQueuedToken( mappedValue, hasMapping := getMapping(svc.Config, svc.DB, svc.Platform, t) scriptText := t.Text if hasMapping { + // The token's own text was bounded above, but the override that + // replaces it was not: a config or platform mapping never reaches the + // API's check. Reject before the parse below, and before history is + // written, exactly as an over-long token is. + if lenErr := zapscript.ValidateScriptLength(mappedValue); lenErr != nil { + rejectOversizedToken(&t, lenErr) + return + } scriptText = mappedValue } diff --git a/pkg/service/readers.go b/pkg/service/readers.go index 6d75a4c66..448f4f7e5 100644 --- a/pkg/service/readers.go +++ b/pkg/service/readers.go @@ -37,6 +37,7 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/state" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/tokens" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/zapscript" uievents "github.com/ZaparooProject/zaparoo-core/v2/pkg/ui/events" "github.com/jonboulle/clockwork" "github.com/rs/zerolog/log" @@ -908,8 +909,14 @@ preprocessing: if hasMapping { scriptText = mappedValue } - parser := gozapscript.NewParser(scriptText) - script, parseErr := parser.ParseScript() + // The scan was bounded above, but a mapping override was not, + // and the queue rejects the token either way. Skip the parse + // so an over-long override cannot be expensive here. + parseErr := zapscript.ValidateScriptLength(scriptText) + var script gozapscript.Script + if parseErr == nil { + script, parseErr = gozapscript.NewParser(scriptText).ParseScript() + } // Stage conservatively: if parsing fails we can't confirm the token // is a safe utility command, so stage it. Only pass through tokens diff --git a/pkg/service/scan_behavior_test.go b/pkg/service/scan_behavior_test.go index dcf43ad10..d16b2905a 100644 --- a/pkg/service/scan_behavior_test.go +++ b/pkg/service/scan_behavior_test.go @@ -22,6 +22,7 @@ package service import ( "context" "path/filepath" + "strconv" "sync" "testing" "time" @@ -40,6 +41,7 @@ import ( testhelpers "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/helpers" "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/mocks" "github.com/jonboulle/clockwork" + "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -59,6 +61,7 @@ type scanBehaviorEnv struct { svc *ServiceContext launchHook *launchHook historyHook *historyHook + fs afero.Fs scanQueue chan readers.Scan itq chan tokens.Token clock *clockwork.FakeClock @@ -282,6 +285,7 @@ mode = "unrestricted"`)) st: st, cfg: cfg, userDB: mockUserDB, + fs: fs.Fs, svc: svc, launchHook: hook, historyHook: histHook, @@ -1345,3 +1349,16 @@ scan_mode = "hold"`)) env.expectNoStop(t) } } + +// addConfigMapping installs a mapping through a mappings TOML file, which is +// the source that never passes through the API's validation. +func (env *scanBehaviorEnv) addConfigMapping(t *testing.T, pattern, script string) { + t.Helper() + + dir := t.TempDir() + toml := "[[mappings.entry]]\ntoken_key = \"value\"\nmatch_pattern = " + + strconv.Quote(pattern) + "\nzapscript = " + strconv.Quote(script) + "\n" + require.NoError(t, env.fs.MkdirAll(dir, 0o750)) + require.NoError(t, afero.WriteFile(env.fs, filepath.Join(dir, "test.toml"), []byte(toml), 0o600)) + require.NoError(t, env.cfg.LoadMappings(dir)) +} diff --git a/pkg/service/token_completion_test.go b/pkg/service/token_completion_test.go index c3efbb36a..f27bb7e8f 100644 --- a/pkg/service/token_completion_test.go +++ b/pkg/service/token_completion_test.go @@ -370,3 +370,24 @@ func TestTokenForLog_DropsCompletion(t *testing.T) { assert.Nil(t, tokenForLog(&tok).Completion) assert.NotNil(t, tok.Completion, "the caller's token must be left alone") } + +// A mapping override replaces the token's text after that text was bounded, +// and a config or platform mapping never passed through the API's check at +// all, so the substituted script has to be bounded where it is applied. +func TestTokenCompletion_OversizedMappingOverrideIsRejected(t *testing.T) { + t.Parallel() + env := setupScanBehavior(t, "tap", 0) + + env.addConfigMapping(t, "mapme", "**echo:"+strings.Repeat("A", zapscript.MaxScriptLength)) + + c := env.sendAPIToken(t, "mapme") + + require.ErrorIs(t, assertCompletedOnce(t, c), zapscript.ErrScriptTooLong) + env.expectNoLaunch(t) + + select { + case he := <-env.historyCh: + t.Fatalf("a token with an over-long override was recorded: %d bytes", len(he.TokenValue)) + case <-time.After(noEventWait): + } +} From 950df85a9a9ad2442b0bb4f72436cfec950238e5 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 3 Sep 2026 07:03:26 +0800 Subject: [PATCH 09/14] fix(service): bound and redact a reader scan before it is logged handleQueuedToken is not the first thing a reader's text reaches. The preprocessing loop logs the scan in full at info level and, with the launch guard on, parses it, both before the token is queued. A 300KB tag wrote about 600KB across three log lines and forced two rotations of a log that lives in tmpfs, evicting everything useful about the scan that caused it. The bound sits after the duplicate check so a rejected tag left sitting on the reader is reported once rather than on every poll, and the fail sound gives the same feedback as any other unreadable scan. Hold ownership is unaffected: the token never launches, so it never becomes the software token, and a later removal has nothing to exit. The two info-level lines now go through tokenForLog, which the worker already uses, because a profile or extension card carries a bearer credential in its text and the log is downloadable. --- pkg/service/readers.go | 23 +++++++++++++++--- pkg/service/token_completion_test.go | 35 ++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/pkg/service/readers.go b/pkg/service/readers.go index 448f4f7e5..efb8bc968 100644 --- a/pkg/service/readers.go +++ b/pkg/service/readers.go @@ -37,8 +37,8 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/state" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/tokens" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/zapscript" uievents "github.com/ZaparooProject/zaparoo-core/v2/pkg/ui/events" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/zapscript" "github.com/jonboulle/clockwork" "github.com/rs/zerolog/log" ) @@ -832,6 +832,20 @@ preprocessing: continue preprocessing case scanNewToken: + // A reader's text is bounded here rather than only at the token + // queue, because the scan is logged in full, and parsed by the + // launch guard, before it ever gets there. Deduplication has + // already run, so a rejected tag left sitting on the reader is + // reported once rather than on every poll. + if lenErr := zapscript.ValidateScriptLength(scan.Text); lenErr != nil { + log.Warn().Err(lenErr). + Str("readerID", scanReaderID). + Int("length", len(scan.Text)). + Msg("ignoring scan, script exceeds maximum length") + playFail() + continue preprocessing + } + delete(pendingRemovals, newHoldTokenKey(scan)) // Suppress the first scan from each newly-connected reader when ignore_on_connect is enabled @@ -846,7 +860,10 @@ preprocessing: connectScanSeen[scan.ReaderID] = true } - log.Info().Msgf("new token scanned: %v", scan) + // A profile or extension card carries a bearer credential in its + // text, so it is redacted here for the same reason it is on the + // worker: the log is downloadable and this line is at info level. + log.Info().Msgf("new token scanned: %v", tokenForLog(scan)) // Run on_scan hook before SetActiveCard so last_scanned refers to previous token if onScanScript := svc.Config.ReadersScan().OnScan; onScanScript != "" { @@ -971,7 +988,7 @@ preprocessing: } } - log.Info().Msgf("sending token to queue: %v", scan) + log.Info().Msgf("sending token to queue: %v", tokenForLog(scan)) select { case itq <- *scan: case <-svc.State.GetContext().Done(): diff --git a/pkg/service/token_completion_test.go b/pkg/service/token_completion_test.go index f27bb7e8f..6643c187e 100644 --- a/pkg/service/token_completion_test.go +++ b/pkg/service/token_completion_test.go @@ -371,6 +371,41 @@ func TestTokenForLog_DropsCompletion(t *testing.T) { assert.NotNil(t, tok.Completion, "the caller's token must be left alone") } +// A reader's text is bounded in the reader loop, not only at the token queue. +// The scan is logged in full and parsed by the launch guard before it reaches +// the queue, so an unbounded tag would flood a tmpfs log and pay for a parse +// on the reader goroutine whatever the queue later decided. +func TestScanBehavior_OversizedReaderScanIsIgnored(t *testing.T) { + t.Parallel() + env := setupScanBehavior(t, "tap", 0) + + env.sendCommandScan("big", "**echo:"+strings.Repeat("A", zapscript.MaxScriptLength)) + env.expectNoLaunch(t) + + select { + case he := <-env.historyCh: + t.Fatalf("an over-long scan was recorded in history: %d bytes", len(he.TokenValue)) + case <-time.After(noEventWait): + } + + // The reader loop must still be running for the next scan. + nextPath := env.gamePath("game1.gba") + env.sendGameScan("ok", nextPath) + assert.Equal(t, nextPath, env.waitForLaunch(t)) +} + +// A scan right on the limit is ordinary input and must still launch. +func TestScanBehavior_ReaderScanAtLengthLimitIsAccepted(t *testing.T) { + t.Parallel() + env := setupScanBehavior(t, "tap", 0) + + path := env.gamePath("game1.gba") + pad := strings.Repeat("A", zapscript.MaxScriptLength-len(path)-len("||**echo:")) + env.sendCommandScan("edge", path+"||**echo:"+pad) + + assert.Equal(t, path, env.waitForLaunch(t)) +} + // A mapping override replaces the token's text after that text was bounded, // and a config or platform mapping never passed through the API's check at // all, so the substituted script has to be bounded where it is applied. From 17b411cc0c733879353a7e4a91f86a06000ee59e Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 3 Sep 2026 07:03:40 +0800 Subject: [PATCH 10/14] fix(api): report an over-long script as an invalid script Every other reason a script will not run answers with a category, and the documentation tells clients to branch on it. The length rejection returned none, so a client switching on data.category fell into its generic branch for a failure it could have explained. invalid_script is reused rather than adding a category, so a client already handling the existing set needs no change; the message carries the limit and the actual size. runError maps the error too, which is what NFC normalization needs: text under the bound can grow past it, `**echo:` plus 2700 U+0958 goes from 8107 to 16207 bytes, and that is caught by the queue rather than at the handler. --- pkg/api/methods/run.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/pkg/api/methods/run.go b/pkg/api/methods/run.go index cba6a17ca..8d812b5d2 100644 --- a/pkg/api/methods/run.go +++ b/pkg/api/methods/run.go @@ -90,7 +90,7 @@ func HandleRun(env requests.RequestEnv) (any, error) { //nolint:gocritic // sing // below. if params.Text != nil { if lenErr := zapscript.ValidateScriptLength(*params.Text); lenErr != nil { - return nil, models.ClientErr(lenErr) + return nil, scriptTooLongErr(lenErr) } } @@ -140,7 +140,7 @@ func HandleRun(env requests.RequestEnv) (any, error) { //nolint:gocritic // sing } if lenErr := zapscript.ValidateScriptLength(text); lenErr != nil { - return nil, models.ClientErr(lenErr) + return nil, scriptTooLongErr(lenErr) } t.Text = norm.NFC.String(text) @@ -210,6 +210,14 @@ func runContextError(env *requests.RequestEnv, ctxErr error) error { } } +// scriptTooLongErr categorizes the length rejection as an invalid script. +// Every other reason a script will not run reports that category, and reusing +// it means a client already branching on the category handles this without a +// change; the message says which limit was exceeded. +func scriptTooLongErr(err error) error { + return models.CategorizedErr(models.ErrorCategoryInvalidScript, err.Error(), err) +} + // runError maps a terminal execution error onto a stable category with a // message that carries no filesystem paths or token contents. The cause is // kept for logging and errors.Is. @@ -227,6 +235,10 @@ func runError(err error) error { case errors.Is(err, state.ErrRunZapScriptDisabled): return models.CategorizedErr(models.ErrorCategoryDisabled, "ZapScript execution is disabled", err) + case errors.Is(err, zapscript.ErrScriptTooLong): + // The queue's backstop rejects a token the API bound never saw, such + // as one whose mapping override replaced its text. + return scriptTooLongErr(err) case errors.Is(err, zapscript.ErrInvalidScript), errors.Is(err, zapscript.ErrUnknownCommand), errors.Is(err, systemdefs.ErrUnknownSystem), From 3405068866ca72d3b0f35c29627297f0c18b16a2 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 3 Sep 2026 07:03:52 +0800 Subject: [PATCH 11/14] fix(userdb): stop logging the switch ID a profile lookup missed A switch ID is the bearer credential printed on a profile card. When a lookup found no row the error quoted the value it searched for, and that error is logged at error level on every failed profile or extension card, so the credential landed in the log in the clear regardless of the redaction applied everywhere else. Only the SwitchID column is treated this way. The other lookups take a profile ID, which is not a secret and is worth naming. --- pkg/database/userdb/profiles.go | 6 ++++++ pkg/database/userdb/profiles_test.go | 3 +++ 2 files changed, 9 insertions(+) diff --git a/pkg/database/userdb/profiles.go b/pkg/database/userdb/profiles.go index 5c8d3c41a..d9f8d89c4 100644 --- a/pkg/database/userdb/profiles.go +++ b/pkg/database/userdb/profiles.go @@ -150,6 +150,12 @@ func sqlGetProfile(ctx context.Context, db *sql.DB, column, value string) (*data p, err := scanProfile(row.Scan) if err != nil { if errors.Is(err, sql.ErrNoRows) { + // A switch ID is the bearer credential on a profile card, and + // this error is logged, so name the column that missed without + // quoting what was looked up. + if column == "SwitchID" { + return nil, fmt.Errorf("%w: no profile for the given %s", ErrProfileNotFound, column) + } return nil, fmt.Errorf("%w: %s=%s", ErrProfileNotFound, column, value) } return nil, fmt.Errorf("failed to scan profile row: %w", err) diff --git a/pkg/database/userdb/profiles_test.go b/pkg/database/userdb/profiles_test.go index e74a7d871..a161b806e 100644 --- a/pkg/database/userdb/profiles_test.go +++ b/pkg/database/userdb/profiles_test.go @@ -144,6 +144,9 @@ func TestProfiles_NotFoundErrors(t *testing.T) { _, err = db.GetProfileBySwitchID("missing-switch") require.ErrorIs(t, err, ErrProfileNotFound) + // A switch ID is a bearer credential and this error is logged, so it + // must not carry the value that was looked up. + require.NotContains(t, err.Error(), "missing-switch") err = db.UpdateProfile(newTestProfile("missing", "a-b-c")) require.ErrorIs(t, err, ErrProfileNotFound) From d90638eeedca9d039ce1b4bdc2bd8f261c54b5b5 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 3 Sep 2026 07:04:19 +0800 Subject: [PATCH 12/14] fix(readers): redact and bound the token text drivers log Every driver logs raw tag text: the file driver on each new token, pn532 and libnfc on each decoded NDEF record and on each record they write. None of it is redacted, so a profile card leaks its switch ID, and none of it is bounded, because the driver runs before anything has decided the text is a reasonable length. ForLog answers both. It redacts, and it truncates at MaxScriptLength reporting the real size, cutting on a rune boundary so the line stays valid UTF-8. A legitimate script is never shortened, since a longer one is rejected rather than run, so the only text this affects is text that was going to do nothing except fill the log. The write paths matter as much as the read paths: writing a profile card is exactly the case where the text being logged is a credential. --- pkg/readers/file/file.go | 3 ++- pkg/readers/libnfc/libnfc.go | 5 ++-- pkg/readers/pn532/pn532.go | 5 ++-- pkg/zapscript/redact.go | 23 ++++++++++++++++ pkg/zapscript/redact_test.go | 51 ++++++++++++++++++++++++++++++++++++ 5 files changed, 82 insertions(+), 5 deletions(-) diff --git a/pkg/readers/file/file.go b/pkg/readers/file/file.go index 0249de453..6eb351842 100644 --- a/pkg/readers/file/file.go +++ b/pkg/readers/file/file.go @@ -34,6 +34,7 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/syncutil" "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/tokens" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/zapscript" "github.com/rs/zerolog/log" ) @@ -212,7 +213,7 @@ func (r *Reader) Open(device config.ReadersConnect, iq chan<- readers.Scan, _ re ReaderID: r.ReaderID(), } - log.Debug().Msgf("new token: %s", token.Text) + log.Debug().Msgf("new token: %s", zapscript.ForLog(token.Text)) iq <- readers.Scan{ Source: tokens.SourceReader, ReaderID: r.ReaderID(), diff --git a/pkg/readers/libnfc/libnfc.go b/pkg/readers/libnfc/libnfc.go index f85328e4c..20e0ba5f3 100644 --- a/pkg/readers/libnfc/libnfc.go +++ b/pkg/readers/libnfc/libnfc.go @@ -20,6 +20,7 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers/libnfc/tags" "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers/shared/ndef" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/tokens" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/zapscript" "github.com/clausecker/nfc/v2" "github.com/rs/zerolog/log" ) @@ -851,7 +852,7 @@ func (r *Reader) pollDevice( if tagText == "" { log.Warn().Msg("no text NDEF found") } else { - log.Debug().Msgf("decoded text NDEF: %s", tagText) + log.Debug().Msgf("decoded text NDEF: %s", zapscript.ForLog(tagText)) } card := &tokens.Token{ @@ -869,7 +870,7 @@ func (r *Reader) pollDevice( func (r *Reader) writeTag(req *WriteRequest) { log.Info().Msg("libnfc write request received") - log.Debug().Msgf("libnfc write text: %s", req.Text) + log.Debug().Msgf("libnfc write text: %s", zapscript.ForLog(req.Text)) r.mu.RLock() pnd := r.pnd diff --git a/pkg/readers/pn532/pn532.go b/pkg/readers/pn532/pn532.go index 9128b46b8..4eaef0151 100644 --- a/pkg/readers/pn532/pn532.go +++ b/pkg/readers/pn532/pn532.go @@ -44,6 +44,7 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/syncutil" "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/tokens" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/zapscript" "github.com/rs/zerolog" "github.com/rs/zerolog/log" ) @@ -532,7 +533,7 @@ func (r *Reader) processNewTag(ctx context.Context, detectedTag *pn532.DetectedT log.Info().Msgf("detected %s tag: %s", token.Type, token.UID) if token.Text != "" { - log.Debug().Msgf("NDEF text: %s", token.Text) + log.Debug().Msgf("NDEF text: %s", zapscript.ForLog(token.Text)) } iq <- readers.Scan{ @@ -867,7 +868,7 @@ func (r *Reader) WriteTarget(ctx context.Context, text string, opts readers.Writ } log.Info().Msg("successfully wrote text to PN532 tag") - log.Debug().Msgf("wrote NDEF text: %s", text) + log.Debug().Msgf("wrote NDEF text: %s", zapscript.ForLog(text)) // Create result token with UID from the tag tagType := tag.Type() diff --git a/pkg/zapscript/redact.go b/pkg/zapscript/redact.go index 5e188fd41..a8a5bfd7c 100644 --- a/pkg/zapscript/redact.go +++ b/pkg/zapscript/redact.go @@ -20,7 +20,9 @@ package zapscript import ( + "fmt" "strings" + "unicode/utf8" gozapscript "github.com/ZaparooProject/go-zapscript" ) @@ -261,3 +263,24 @@ func RedactToken(text, data string) (redactedText, redactedData string) { } return redactedText, data } + +// ForLog returns text safe to write to a log line: bearer credentials +// replaced, and anything longer than MaxScriptLength truncated. +// +// The truncation is what makes this usable from a reader driver, which logs +// whatever a tag holds before anything has bounded it. A legitimate script is +// never affected, because a longer one is rejected rather than run, so the +// only text this shortens is text that was never going to do anything except +// fill a log that lives in tmpfs. +func ForLog(text string) string { + if len(text) <= MaxScriptLength { + return RedactScript(text) + } + // Cut on a rune boundary so the log line stays valid UTF-8. Redaction + // runs on the kept portion, which is the only part that reaches the log. + cut := MaxScriptLength + for cut > 0 && !utf8.RuneStart(text[cut]) { + cut-- + } + return fmt.Sprintf("%s\u2026 (%d bytes)", RedactScript(text[:cut]), len(text)) +} diff --git a/pkg/zapscript/redact_test.go b/pkg/zapscript/redact_test.go index 09dac9cdc..9091ce8f6 100644 --- a/pkg/zapscript/redact_test.go +++ b/pkg/zapscript/redact_test.go @@ -22,6 +22,7 @@ package zapscript import ( "strings" "testing" + "unicode/utf8" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -337,3 +338,53 @@ func TestRedactScript_RemovesEveryCaseVariantOfTheProfileArg(t *testing.T) { assert.NotContains(t, got, testSwitchID) assert.NotContains(t, got, second) } + +// ForLog is what a reader driver calls on text nothing has bounded yet, so it +// has to both redact and cap. A script within the limit is untouched beyond +// redaction; a longer one is cut, and its real size reported. +func TestForLog(t *testing.T) { + t.Parallel() + + t.Run("ordinary text is unchanged", func(t *testing.T) { + t.Parallel() + text := "**launch:/games/snes/mario.sfc" + assert.Equal(t, text, ForLog(text)) + }) + + t.Run("credential is redacted", func(t *testing.T) { + t.Parallel() + got := ForLog("**profile:" + testSwitchID) + assert.NotContains(t, got, testSwitchID) + assert.Contains(t, got, RedactedPlaceholder) + }) + + t.Run("text at the limit is not truncated", func(t *testing.T) { + t.Parallel() + text := "**echo:" + strings.Repeat("A", MaxScriptLength-len("**echo:")) + require.Len(t, text, MaxScriptLength) + assert.Equal(t, text, ForLog(text)) + }) + + t.Run("longer text is cut and its size reported", func(t *testing.T) { + t.Parallel() + text := "**echo:" + strings.Repeat("A", 300000) + got := ForLog(text) + assert.Less(t, len(got), MaxScriptLength+64, "a log line must not carry the whole payload") + assert.Contains(t, got, "(300007 bytes)") + }) + + t.Run("a cut never splits a rune", func(t *testing.T) { + t.Parallel() + // Three-byte runes do not divide evenly into the limit, so the cut + // lands mid-rune unless it is moved back to a boundary. + text := "**echo:" + strings.Repeat("あ", 4000) + got := ForLog(text) + assert.True(t, utf8.ValidString(got), "log text must stay valid UTF-8") + }) + + t.Run("a credential inside the kept portion still goes", func(t *testing.T) { + t.Parallel() + text := "**profile:" + testSwitchID + "||**echo:" + strings.Repeat("A", 300000) + assert.NotContains(t, ForLog(text), testSwitchID) + }) +} From df80d44ec2959cb7437209c71b04ee8bcd1191be Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 3 Sep 2026 07:04:26 +0800 Subject: [PATCH 13/14] docs(api): document the ZapScript length limit The bound is client visible: run rejects text over it, the launch endpoint answers 413, and a mapping override is refused. None of that was written down, and the invalid_script row did not list length as a reason a script is refused. --- docs/api/index.md | 2 ++ docs/api/methods.md | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/api/index.md b/docs/api/index.md index 07df2a1af..038808c3d 100644 --- a/docs/api/index.md +++ b/docs/api/index.md @@ -260,6 +260,8 @@ Requests from the local device are allowed without restriction. Remote requests These endpoints respond as soon as the token is accepted and do not report execution failures. Use the JSON-RPC [`run`](methods.md#run) method to wait for execution and receive its result. +The decoded ZapScript may be at most 8192 bytes. A longer path returns `413 Request Entity Too Large` and nothing is run. + ## Methods Methods execute actions and return data from Core. See [API Methods](./methods) for request and response contracts, complete access details, and examples. **Local/admin** means localhost or authenticated admin, including paired and valid static API-key admins; **Tiered** means fields or availability vary by client and are detailed in method reference. diff --git a/docs/api/methods.md b/docs/api/methods.md index a51c1c88f..344945d13 100644 --- a/docs/api/methods.md +++ b/docs/api/methods.md @@ -38,7 +38,7 @@ Accepts two types of parameters: | :----- | :------ | :------- | :------------------------------------------------------------------------------------------------------------- | | type | string | No | An internal category of the type of token being scanned. _Not currently in use outside of logging._ | | uid | string | No\* | The UID of the token being scanned. For example, the UID of an NFC tag. Used for matching mappings. | -| text | string | No\* | The main text to be processed from a scan, should contain [ZapScript](../../zapscript/index.md). | +| text | string | No\* | The main text to be processed from a scan, should contain [ZapScript](../../zapscript/index.md). At most 8192 bytes. | | data | string | No\* | The raw data read from a token, converted to a hexadecimal string. Used in mappings and detection of NFC toys. | | unsafe | boolean | No | Allow unsafe operations. Default is false. | @@ -55,7 +55,7 @@ If execution fails, the response carries an [error](index.md#response-errors) wh | `busy` | Another launch is already in progress. | | `media_not_found` | The requested media could not be found or matched. | | `disabled` | ZapScript execution is disabled in settings. | -| `invalid_script` | The script could not be parsed, or names an unknown command or system. | +| `invalid_script` | The script could not be parsed, names an unknown command or system, or exceeds 8192 bytes. | | `blocked` | Execution was refused by configuration, a profile requirement or a hook. | | `playtime_limit` | A playtime limit prevented the launch. | | `timeout` | Core stopped waiting after the request timeout (30 seconds). Anything already started continues. | @@ -4191,7 +4191,7 @@ An object: | type | string | Yes | The field which will be matched against:
_ `uid`: match on UID, if available. UIDs are normalized before matching to remove spaces, colons and convert to lowercase.
_ `text`: match on the stored text on token.
\* `data`: match on the raw token data, if available. This is converted from bytes to a hexadecimal string and should be matched as this. | | match | string | Yes | The method used to match a mapping pattern:
_ `exact`: match the entire string exactly to the field.
_ `partial`: match part of the string to the field.
\* `regex`: use a regular expression to match the field. | | pattern | string | Yes | Pattern that will be matched against the token, using the above settings. | -| override | string | Yes | Final text that will completely replace the existing token text if a match was successful. | +| override | string | Yes | Final text that will completely replace the existing token text if a match was successful. At most 8192 bytes. | #### Result @@ -4288,7 +4288,7 @@ An object: | type | string | No | The field which will be matched against:
_ `uid`: match on UID, if available. UIDs are normalized before matching to remove spaces, colons and convert to lowercase.
_ `text`: match on the stored text on token.
\* `data`: match on the raw token data, if available. This is converted from bytes to a hexadecimal string and should be matched as this. | | match | string | No | The method used to match a mapping pattern:
_ `exact`: match the entire string exactly to the field.
_ `partial`: match part of the string to the field.
\* `regex`: use a regular expression to match the field. | | pattern | string | No | Pattern that will be matched against the token, using the above settings. | -| override | string | No | Final text that will completely replace the existing token text if a match was successful. | +| override | string | No | Final text that will completely replace the existing token text if a match was successful. At most 8192 bytes. | Only keys which are provided in the object will be updated in the database. From 8a4984a09cfa36fb32fa718fee5d250e3c75890c Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 3 Sep 2026 07:16:27 +0800 Subject: [PATCH 14/14] fix(readers): only build the log text when debug logging is on ForLog was an argument to Msgf, so it was evaluated before zerolog could decide the event was disabled. Every scan and every tag write paid for a redaction scan of the text, and an oversized one paid for a truncation too, with debug logging off. This is the same mistake runParamsForLog carried, and it takes the same shape as the fix there. --- pkg/readers/file/file.go | 4 +++- pkg/readers/libnfc/libnfc.go | 8 ++++++-- pkg/readers/pn532/pn532.go | 8 ++++++-- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/pkg/readers/file/file.go b/pkg/readers/file/file.go index 6eb351842..e18d88711 100644 --- a/pkg/readers/file/file.go +++ b/pkg/readers/file/file.go @@ -213,7 +213,9 @@ func (r *Reader) Open(device config.ReadersConnect, iq chan<- readers.Scan, _ re ReaderID: r.ReaderID(), } - log.Debug().Msgf("new token: %s", zapscript.ForLog(token.Text)) + if e := log.Debug(); e.Enabled() { + e.Msgf("new token: %s", zapscript.ForLog(token.Text)) + } iq <- readers.Scan{ Source: tokens.SourceReader, ReaderID: r.ReaderID(), diff --git a/pkg/readers/libnfc/libnfc.go b/pkg/readers/libnfc/libnfc.go index 20e0ba5f3..381f24e1e 100644 --- a/pkg/readers/libnfc/libnfc.go +++ b/pkg/readers/libnfc/libnfc.go @@ -852,7 +852,9 @@ func (r *Reader) pollDevice( if tagText == "" { log.Warn().Msg("no text NDEF found") } else { - log.Debug().Msgf("decoded text NDEF: %s", zapscript.ForLog(tagText)) + if e := log.Debug(); e.Enabled() { + e.Msgf("decoded text NDEF: %s", zapscript.ForLog(tagText)) + } } card := &tokens.Token{ @@ -870,7 +872,9 @@ func (r *Reader) pollDevice( func (r *Reader) writeTag(req *WriteRequest) { log.Info().Msg("libnfc write request received") - log.Debug().Msgf("libnfc write text: %s", zapscript.ForLog(req.Text)) + if e := log.Debug(); e.Enabled() { + e.Msgf("libnfc write text: %s", zapscript.ForLog(req.Text)) + } r.mu.RLock() pnd := r.pnd diff --git a/pkg/readers/pn532/pn532.go b/pkg/readers/pn532/pn532.go index 4eaef0151..aff9df662 100644 --- a/pkg/readers/pn532/pn532.go +++ b/pkg/readers/pn532/pn532.go @@ -533,7 +533,9 @@ func (r *Reader) processNewTag(ctx context.Context, detectedTag *pn532.DetectedT log.Info().Msgf("detected %s tag: %s", token.Type, token.UID) if token.Text != "" { - log.Debug().Msgf("NDEF text: %s", zapscript.ForLog(token.Text)) + if e := log.Debug(); e.Enabled() { + e.Msgf("NDEF text: %s", zapscript.ForLog(token.Text)) + } } iq <- readers.Scan{ @@ -868,7 +870,9 @@ func (r *Reader) WriteTarget(ctx context.Context, text string, opts readers.Writ } log.Info().Msg("successfully wrote text to PN532 tag") - log.Debug().Msgf("wrote NDEF text: %s", zapscript.ForLog(text)) + if e := log.Debug(); e.Enabled() { + e.Msgf("wrote NDEF text: %s", zapscript.ForLog(text)) + } // Create result token with UID from the tag tagType := tag.Type()