fix(zapscript): bound ZapScript length and stop re-parsing every token

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.

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:<br/>_ `uid`: match on UID, if available. UIDs are normalized before matching to remove spaces, colons and convert to lowercase.<br/>_ `text`: match on the stored text on token.<br/>\* `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:<br/>_ `exact`: match the entire string exactly to the field.<br/>_ `partial`: match part of the string to the field.<br/>\* `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:<br/>_ `uid`: match on UID, if available. UIDs are normalized before matching to remove spaces, colons and convert to lowercase.<br/>_ `text`: match on the stored text on token.<br/>\* `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:<br/>_ `exact`: match the entire string exactly to the field.<br/>_ `partial`: match part of the string to the field.<br/>\* `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.
 

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

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=

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
 	}
 

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)
+}

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()
 

pkg/api/methods/run.go

@@ -86,7 +86,17 @@ func HandleRun(env requests.RequestEnv) (any, error) { //nolint:gocritic // sing
 			return nil, models.ClientErrf("invalid params: %w", err)
 		}
 
-		log.Debug().Msgf("unmarshalled run params: %+v", runParamsForLog(&params))
+		// 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, scriptTooLongErr(lenErr)
+			}
+		}
+
+		if e := log.Debug(); e.Enabled() {
+			e.Msgf("unmarshalled run params: %+v", runParamsForLog(&params))
+		}
 
 		if params.Type != nil {
 			t.Type = *params.Type
@@ -129,6 +139,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, scriptTooLongErr(lenErr)
+		}
+
 		t.Text = norm.NFC.String(text)
 	}
 
@@ -196,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.
@@ -213,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),
@@ -268,6 +294,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)

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()