Release 12/8/2025

[DJAndries]

No description provided.

[github-actions]

The following commits were not verified:
7247727 (unsigned)

[github-actions]

[puLL-Merge] - brave/go-sync@391

Pull Request Review

Description

This PR performs a major refactoring and dependency upgrade:

1. Splits CI workflow into separate lint and test jobs
2. Migrates from AWS SDK v1 to v2 - a significant architectural change requiring substantial code updates
3. Updates dependencies including Go 1.25, Redis client, Sentry, and GitHub Actions
4. Refactors code structure:
   • Renames context package to synccontext to avoid Go stdlib conflict
   • Replaces fmt.Errorf with errors.New where appropriate
   • Switches from satori/go.uuid to google/uuid
   • Adds device limit functionality with configurable high-limit client IDs
5. Improves code quality with better linting, assertion ordering, and nolint directives

Possible Issues

1. AWS SDK v2 Migration Risk: This is a breaking change that fundamentally alters how AWS services are accessed. The PR changes from session-based to config-based initialization, which could have runtime implications:

   • Context is now required for all DynamoDB operations
   • Pagination patterns have changed significantly
   • Error handling has shifted from type assertions to errors.As()

2. Device Limit Logic: The new device limit feature (device_limit.go) uses a global map initialized at startup. If HIGH_DEVICE_LIMIT_CLIENT_IDS changes at runtime, it won't be reflected without restart.

3. Context Propagation: Many DynamoDB calls now use context.TODO() instead of properly propagated contexts, which could make cancellation and timeouts less effective.

4. BatchGetItem Pagination: The migration to AWS SDK v2's paginator changes the behavior significantly. The old code used BatchGetItemPages callback pattern; the new code uses NextPage() in a loop, which could behave differently with UnprocessedKeys.

Security Hotspots

1. Environment Variable Injection (command/device_limit.go:18-20): The HIGH_DEVICE_LIMIT_CLIENT_IDS environment variable is loaded without validation. Malicious input could potentially bypass device limits:

   clientIDsEnv := os.Getenv("HIGH_DEVICE_LIMIT_CLIENT_IDS")
   LoadHighDeviceLimitClientIDs(clientIDsEnv)

   Consider validating client IDs match expected format (e.g., hex strings of specific length).

2. Client ID Case Sensitivity (command/device_limit.go:27): The device limit check converts client IDs to lowercase, but it's unclear if this is consistently applied throughout the codebase. Inconsistent case handling could allow device limit bypass through case variation.

Privacy Hotspots

1. Client ID Logging: Throughout the codebase, client IDs are logged in various places (e.g., error messages in datastore/sync_entity.go). While the PR doesn't introduce new logging, the migration to AWS SDK v2 preserves these patterns. Consider whether client IDs should be treated as PII and redacted from logs.

2. Device Limit Environment Variable: The HIGH_DEVICE_LIMIT_CLIENT_IDS contains a list of specific client IDs that get special treatment. This list could potentially leak information about which clients/users have elevated privileges if the environment configuration is exposed.

Changes

.github/workflows/ci.yml

• Splits single ci job into separate lint and test jobs for better parallelization
• Updates GitHub Actions versions (checkout v6.0.0, setup-go v6.1.0, golangci-lint v9.1.0)
• Bumps Go version to 1.25

auth/auth.go & auth/auth_test.go

• Replaces fmt.Errorf with errors.New for static error messages (no formatting needed)
• Adds errors import
• Adds slices package usage to simplify blocked client ID checking
• Improves assertion order in tests (expected, actual)

cache/cache.go & cache/cache_test.go

• Fixes comment typo
• Improves test assertion ordering

command/command.go

• Major refactoring to use AWS SDK v2 types
• Replaces fmt.Errorf with errors.New for static errors
• Adds device limit checking logic via hasReachedDeviceLimit()
• Capitalizes error messages consistently
• Imports reorganization

command/device_limit.go (NEW)

• Implements configurable device limits (50 default, 100 for whitelisted clients)
• Loads high-limit client IDs from HIGH_DEVICE_LIMIT_CLIENT_IDS environment variable
• Provides hasReachedDeviceLimit() function for limit checks

command/command_test.go

• Adds comprehensive test for device limit functionality (TestHandleClientToServerMessage_DeviceLimitExceeded)
• Migrates to AWS SDK v2 types
• Improves test assertions

datastore/dynamo.go

• Complete rewrite for AWS SDK v2:
  • Changes from session.NewSession() to config.LoadDefaultConfig()
  • Uses dynamodb.NewFromConfig() instead of dynamodb.New()
  • Wraps dynamodb.Client instead of dynamodb.DynamoDB
  • Endpoint configuration now via BaseEndpoint option

datastore/datastoretest/dynamo.go

• Migrates to AWS SDK v2:
  • Uses context for all operations
  • Switches to types.ResourceNotFoundException from string comparison
  • Implements waiter pattern for table creation/deletion
  • Changes dynamodbattribute to attributevalue
• Replaces ioutil.ReadFile with os.ReadFile

datastore/sync_entity.go

• Comprehensive AWS SDK v2 migration:
  • All DB operations now require context (uses context.TODO() throughout)
  • Type assertions replaced with errors.As()
  • Attribute value handling via SDK v2 patterns
  • Transaction handling updated
  • Batch operations use paginator pattern
• Switches from satori/go.uuid to google/uuid
• Improves error messages and simplifies min/max logic

datastore/item_count.go

• Migrates query operations to SDK v2
• Uses types.SelectCount instead of string pointer
• Updates attribute marshaling

controller/controller.go

• Renames import from context to synccontext
• Replaces ioutil.ReadAll with io.ReadAll

middleware/ files

• Updates imports to use synccontext instead of context

server/server.go & server/server_test.go

• Updates imports for synccontext
• Adds //nolint directives for acceptable errors during shutdown
• Replaces ioutil with io

context/context.go → synccontext/context.go

• Renamed package to avoid conflict with Go's standard context package

Test files throughout

• Improves assertion ordering (expected before actual)
• Better use of testify methods (suite.Empty(), suite.Len(), etc.)
• Formatting improvements

go.mod & go.sum

• Adds AWS SDK v2 dependencies
• Updates Redis, Sentry, and other dependencies
• Adds google/uuid dependency

sequenceDiagram
    participant Client
    participant Server
    participant Auth
    participant Command
    participant DeviceLimit
    participant DynamoDB
    
    Client->>Server: POST /v2/command/ (with Bearer token)
    Server->>Auth: Authorize(request)
    Auth->>Auth: Validate token signature
    Auth-->>Server: ClientID
    
    Server->>Command: HandleClientToServerMessage
    
    alt GetUpdates with NEW_CLIENT
        Command->>Command: handleGetUpdatesRequest
        Command->>DeviceLimit: hasReachedDeviceLimit(activeDevices, clientID)
        DeviceLimit->>DeviceLimit: Check if clientID in high-limit list
        DeviceLimit-->>Command: true/false
        
        alt Device limit reached
            Command-->>Server: THROTTLED error
        else Within limit
            Command->>DynamoDB: Query(ctx, GetUpdatesInput)
            DynamoDB-->>Command: Updates
            Command->>DynamoDB: BatchGetItem(ctx, keys)
            DynamoDB-->>Command: Entities
        end
    else Commit
        Command->>Command: handleCommitRequest
        Command->>DynamoDB: TransactWriteItems(ctx, items)
        DynamoDB-->>Command: Success/Conflict
    end
    
    Command-->>Server: Response
    Server-->>Client: ClientToServerResponse

Loading

[mihaiplesa]

Has this been deployed after all?