[TT-16337] prevent use of special characters in policyID

[NurayAhmadova]

Description

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

• I ensured that the documentation is up to date
• I explained why this PR updates go.mod in detail with reasoning why it's required
• I would like a code coverage CI quality gate exception and have explained why

Ticket Details

TT-16337

Status	In Dev
Summary	Prevent use of special characters in policy ID

Generated at: 2026-02-24 16:59:44

[github-actions]

API Changes

--- prev.txt	2026-02-24 17:00:13.013097880 +0000
+++ current.txt	2026-02-24 17:00:02.497004053 +0000
@@ -6833,6 +6833,9 @@
 
 	// JWKS holds the configuration for Tyk JWKS functionalities
 	JWKS JWKSConfig `json:"jwks"`
+
+	// DisableCustomIdValidation disables custom id validation and enables legacy behaviour
+	DisableCustomIdValidation bool `json:"disable_custom_id_validation"`
 }
     Config is the configuration object used by Tyk to set up various parameters.
 
@@ -10500,6 +10503,7 @@
 	StorageConnectionHandler *storage.ConnectionHandler
 
 	BundleChecksumVerifier bundleChecksumVerifyFunction
+
 	// Has unexported fields.
 }
 
@@ -13260,6 +13264,8 @@
 
 func Domain(msg string) Error
 
+func Domainf(format string, a ...any) Error
+
 func Infra(msg string) Error
 
 func New(msg string, opts ...Option) Error
@@ -13271,6 +13277,8 @@
 
 func (e Error) Error() string
 
+func (e Error) Is(err error) bool
+
 func (e Error) TypeOf(typ Type) bool
 
 type Option func(*Error)
@@ -13281,6 +13289,45 @@
 	// Has unexported fields.
 }
 
+# Package: ./pkg/identifier
+
+package identifier // import "github.com/TykTechnologies/tyk/pkg/identifier"
+
+
+VARIABLES
+
+var (
+	ErrInvalidCustomId = errpack.Domain("Invalid custom ID: Allowed characters: a-z, A-Z, 0-9, ., _, -, ~")
+)
+
+TYPES
+
+type Custom string
+    Custom (user-defined-identifier)
+
+func (c Custom) String() string
+
+func (c Custom) Validate() error
+
+# Package: ./pkg/validator
+
+package validator // import "github.com/TykTechnologies/tyk/pkg/validator"
+
+
+TYPES
+
+type Option func(*validatorCfg)
+
+func WithDisabledPolicyIdValidation(disabled bool) Option
+
+type ValidateFn func(val reflect.Value) error
+
+type Validator interface {
+	Validate(v any) error
+}
+
+func New(opts ...Option) Validator
+
 # Package: ./regexp
 
 package regexp // import "github.com/TykTechnologies/tyk/regexp"

[probelabs]

This PR introduces a validation mechanism to prevent the use of special characters in policy IDs. A new, reusable validation service is created in the pkg/validator package, which leverages the go-playground/validator/v10 library. This service ensures that policy IDs conform to the regex ^[a-zA-Z0-9.\-_~]+$.

A new configuration flag, DisableCustomIdValidation, is added to allow operators to disable this validation and maintain backward compatibility with legacy systems. The validation is enforced in the gateway's API endpoint for creating and updating policies, which will now return a 400 Bad Request for IDs with invalid characters.

Files Changed Analysis

• New Packages: The core logic is encapsulated in two new packages: pkg/identifier, which defines the valid format for custom IDs, and pkg/validator, which provides a generic validation framework. This promotes code reuse and separation of concerns.
• Gateway Integration: The gateway (gateway/server.go, gateway/api.go) is modified to initialize and use the new validator service during policy creation and updates.
• Configuration: A new feature flag, DisableCustomIdValidation, is added to the main configuration (config/config.go) and the linter schema to control the feature's behavior.
• Testing: Comprehensive tests are added in gateway/api_test.go and a new test file pkg/validator/validator_test.go to verify the validation logic and the functionality of the feature flag.
• Dependencies: go.mod and go.sum are updated to include github.com/go-playground/validator/v10 and its transitive dependencies.

Architecture & Impact Assessment

• What this PR accomplishes: It enhances system stability and security by enforcing a strict, URL-safe character set for policy IDs, preventing potential parsing, storage, or path traversal issues.
• Key technical changes introduced:
  • A new, extensible validation service built around the go-playground/validator/v10 library.
  • A specific identifier.Custom type that encapsulates the validation logic for policy IDs.
  • Integration of the validator into the policy management API (handleAddOrUpdatePolicy).
  • A feature flag (DisableCustomIdValidation) to disable the validation for backward compatibility.
• Affected system components: The primary impact is on the Gateway's policy management API (/tyk/policies). Any clients (e.g., Tyk Dashboard, Tyk Sync, custom scripts) that create or update policies will be affected. Existing policies with non-conforming IDs can still be read and deleted but cannot be updated without correcting the ID.

sequenceDiagram
    participant Client
    participant Gateway API
    participant Validator Service

    Client->>Gateway API: POST /tyk/policies (policy with ID)
    Gateway API->>Validator Service: Validate(policy.ID)
    alt Invalid Policy ID
        Validator Service-->>Gateway API: Return error
        Gateway API-->>Client: 400 Bad Request
    else Valid Policy ID
        Validator Service-->>Gateway API: Return success
        Gateway API->>Gateway API: Save policy
        Gateway API-->>Client: 200 OK
    end

Loading

Scope Discovery & Context Expansion

• The impact of this change is primarily at the API boundary for policy management. The validation is triggered only on create (POST) and update (PUT) operations, ensuring that existing policies with invalid IDs remain accessible via GET requests.
• The new validation framework in pkg/validator is generic. It could be extended to enforce similar ID constraints on other resources, such as API definitions or security keys, to improve consistency and robustness across the platform.
• To further assess the impact, one could search the codebase for other areas where user-defined identifiers are accepted via API calls and evaluate if they would benefit from being validated by this new service.

Metadata

• Review Effort: 3 / 5
• Primary Label: enhancement

---

Powered by Visor from Probelabs

Last updated: 2026-02-24T17:02:16.800Z | Triggered by: pr_updated | Commit: 60751d9

💡 TIP: You can chat with Visor using /visor ask <your question>

[probelabs]

Security Issues (1)

Severity	Location	Issue
🟢 Info	gateway/api.go:1092-1100	The function `removePersistentPolicyById` constructs a file path for deletion using the `id` parameter without any validation. While it is currently only used in tests with hardcoded values, if it were to be reused in production with user-controlled input, it could introduce a path traversal vulnerability, allowing for arbitrary file deletion. 💡 Suggestion To prevent accidental misuse of this function in the future, add a comment clarifying that it is intended for internal or test use only and that the `id` parameter is not sanitized. For example: ```go // removePersistentPolicyById removes a policy from the filesystem. // WARNING: This function is intended for internal use (e.g., test cleanup) only. // The 'id' parameter is not sanitized and should not be taken from user input. func (gw *Gateway) removePersistentPolicyById(id string) error { ```

Architecture Issues (3)

Severity	Location	Issue
🟠 Error	pkg/validator/validator.go:34-85	The introduction of a new validation framework in `pkg/validator`, which wraps `go-playground/validator/v10`, is an overly complex solution for the single use case of validating policy IDs. This approach introduces a new third-party dependency and significant framework code for a problem that could be solved with a much simpler, direct validation call. 💡 Suggestion Consider removing the `pkg/validator` package and the dependency on `go-playground/validator`. The validation can be performed directly in the API handler by calling the `Validate` method on the `identifier.Custom` type. This would greatly simplify the implementation, reduce complexity, and remove an unnecessary dependency. The validation call in `gateway/api.go` could be guarded by the configuration flag directly: // In gateway/api.go handleAddOrUpdatePolicy() if !gw.GetConfig().DisableCustomIdValidation { if err := identifier.Custom(newPol.ID).Validate(); err != nil { log.WithField("id", newPol.ID).WithError(err).Error("Failed to validate policy ID") return apiError(errMsgInvalidPolicyID), http.StatusBadRequest } }
🟢 Info	pkg/errpack/errpack.go:50-53	The `Is(error) bool` method on `errpack.Error` uses an atypical implementation that compares the error's internal type and message string for equality. This is different from the common Go pattern where `Is` checks for a specific sentinel error instance or an error type. This value-based comparison can be misleading for developers and may lead to incorrect usage. While it works for the current use case, it's a potentially confusing design. 💡 Suggestion Add a comment to the `Is` method explaining why value-based comparison is used and what the intended use case is. This will help prevent future misuse and clarify the design intent for other developers maintaining the code.
🟡 Warning	pkg/validator/validator.go:48-52	The framework implements support for a struct tag `custom_id` for validation, but this feature is not used anywhere in the application's codebase. It is only exercised in the package's own tests using a test-only struct. This violates the YAGNI (You Ain't Gonna Need It) principle by adding code and complexity for a feature that is not currently required. 💡 Suggestion If the validation framework is kept, remove the unused `custom_id` struct tag validation logic to simplify the code. The functionality can be added back if and when a concrete use case for it emerges.

✅ Performance Check Passed

No performance issues found – changes LGTM.

Quality Issues (3)

Severity	Location	Issue
🟢 Info	pkg/identifier/custom.go:23	The `Validate` function for a `Custom` identifier returns `nil` for an empty string. If an empty string is not a valid identifier, this should be handled as an error. Allowing empty identifiers could lead to unexpected behavior or data integrity issues in other parts of the system. 💡 Suggestion If an empty ID is invalid, add a check to the `Validate` function to return an error. For example: ```go func (c Custom) Validate() error { if len(c) == 0 { return ErrInvalidCustomId // Or a more specific error } if !validRe.MatchString(string(c)) { return ErrInvalidCustomId } return nil } If an empty ID is acceptable, consider adding a comment to clarify this behavior. </details> </div></td> </tr> <tr> <td>🟢 Info</td> <td><code>pkg/errpack/errpack.go:50</code></td> <td><div>The `Is` method on the `Error` type has a specific implementation that checks for equality of error type and message, rather than the typical behavior of checking for wrapped errors or reference equality. This is not inherently wrong, but it is a non-standard implementation of `Is`. This could be confusing for developers who expect the standard behavior from `errors.Is`. <details><summary>💡 <strong>Suggestion</strong></summary>Consider adding a comment to the `Is` method to explain its specific behavior. This will help other developers understand its purpose and use it correctly. For example: ```go // Is checks if the target error is an errpack.Error with the same type and message. // This provides value-based equality for sentinel errors. func (e Error) Is(err error) bool { ... }
🟡 Warning	gateway/api_test.go:230	The error returned by `ts.Gw.removePersistentPolicyById("invalid@id")` is ignored. While this is in a test cleanup function and the impact is low, it is best practice to handle or at least acknowledge errors. This ensures that test teardown is successful and doesn't hide potential issues. 💡 Suggestion Check the error returned by `removePersistentPolicyById` and fail the test if it's not nil. For example: `assert.NoError(t, ts.Gw.removePersistentPolicyById("invalid@id"))` or `require.NoError(t, ts.Gw.removePersistentPolicyById("invalid@id"))`.

---

Powered by Visor from Probelabs

Last updated: 2026-02-24T17:02:19.938Z | Triggered by: pr_updated | Commit: 60751d9

💡 TIP: You can chat with Visor using /visor ask <your question>

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
85.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud