Skip to content

Add explicit field allowlist in updateProfile to prevent sensitive field modification #536

Description

@DeFiVC

Bug Description

The updateProfile method in src/modules/users/user.service.ts (lines 82-112) allows users to set arbitrary field values that should be controlled. The method spreads data directly, which could include fields not intended for user modification.

Location

src/modules/users/user.service.ts lines 86-90

const [updated] = await db
    .update(users)
    .set({ ...data, updatedAt: new Date() })
    .where(eq(users.id, userId))
    .returning();

The Problem

The UpdateProfileBody type includes fields like credits, isAdmin, stellarAddress, etc. If a malicious user sends a request body with { "credits": 999999, "isAdmin": true }, the Zod schema should reject it, but:

  1. The service method trusts the input completely
  2. If the Zod schema is misconfigured or bypassed, sensitive fields could be modified
  3. There's no explicit allowlist of which fields users can update

Recommended Fix

Explicitly pick only the allowed fields:

async updateProfile(
    userId: string,
    data: UpdateProfileBody,
): Promise<UserProfile> {
    const updateData: Partial<typeof users.$inferInsert> = {
        updatedAt: new Date(),
    };

    // Only allow specific fields to be updated
    if (data.displayName !== undefined) updateData.displayName = data.displayName;
    if (data.background !== undefined) updateData.background = data.background;
    if (data.learningGoal !== undefined) updateData.learningGoal = data.learningGoal;
    if (data.pace !== undefined) updateData.pace = data.pace;
    if (data.language !== undefined) updateData.language = data.language;

    const [updated] = await db
        .update(users)
        .set(updateData)
        .where(eq(users.id, userId))
        .returning();

Acceptance Criteria

  • Explicitly allowlist which fields can be updated by users
  • Prevent modification of sensitive fields (credits, isAdmin, stellarAddress)
  • Document which fields are user-editable

Severity

medium - Security hardening. While Zod should catch this, defense in depth is important for user data modification.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    securitySecurity concern

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions