Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 10 additions & 12 deletions app/common/CryptoUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,16 @@ export function genRSAKeys(): { publicKey: string; privateKey: string } {
const key = generateKeyPairSync('rsa', {
modulusLength: 512,
});
const publicKey = key.publicKey
.export({
type: 'pkcs1',
format: 'pem',
})
.toString('base64');
const privateKey = key.privateKey
.export({
type: 'pkcs1',
format: 'pem',
})
.toString('base64');
// export({ format: 'pem' }) returns string; .toString('base64') is a no-op on string.
// Use type assertion to fix TS2554 on Node 18 type definitions.
const publicKey = key.publicKey.export({
type: 'pkcs1',
format: 'pem',
}) as string;
const privateKey = key.privateKey.export({
type: 'pkcs1',
format: 'pem',
}) as string;
return { publicKey, privateKey };
}

Expand Down
1 change: 1 addition & 0 deletions app/common/constants.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export const BUG_VERSIONS = 'bug-versions';
export const DEVELOPERS_TEAM = 'developers';
export const LATEST_TAG = 'latest';
export const GLOBAL_WORKER = 'GLOBAL_WORKER';
export const PROXY_CACHE_DIR_NAME = 'proxy-cache-packages';
Expand Down
29 changes: 29 additions & 0 deletions app/core/entity/Org.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { EasyData, EntityUtil } from '../util/EntityUtil.ts';
import { Entity, EntityData } from './Entity.ts';

interface OrgData extends EntityData {
orgId: string;
name: string;
description: string;
}

export type CreateOrgData = Omit<EasyData<OrgData, 'orgId'>, 'id' | 'description'> & { description?: string };

export class Org extends Entity {
orgId: string;
name: string;
description: string;

constructor(data: OrgData) {
super(data);
this.orgId = data.orgId;
this.name = data.name;
this.description = data.description ?? '';
}

static create(data: CreateOrgData): Org {
const fullData = { ...data, description: data.description ?? '' };
const newData = EntityUtil.defaultData(fullData, 'orgId');
return new Org(newData);
}
}
31 changes: 31 additions & 0 deletions app/core/entity/OrgMember.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { EasyData, EntityUtil } from '../util/EntityUtil.ts';
import { Entity, EntityData } from './Entity.ts';

interface OrgMemberData extends EntityData {
orgMemberId: string;
orgId: string;
userId: string;
role: 'owner' | 'member';
}

export type CreateOrgMemberData = Omit<EasyData<OrgMemberData, 'orgMemberId'>, 'id'>;

export class OrgMember extends Entity {
orgMemberId: string;
orgId: string;
userId: string;
role: 'owner' | 'member';

constructor(data: OrgMemberData) {
super(data);
this.orgMemberId = data.orgMemberId;
this.orgId = data.orgId;
this.userId = data.userId;
this.role = data.role;
}

static create(data: CreateOrgMemberData): OrgMember {
const newData = EntityUtil.defaultData(data, 'orgMemberId');
return new OrgMember(newData);
}
}
32 changes: 32 additions & 0 deletions app/core/entity/Team.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { EasyData, EntityUtil } from '../util/EntityUtil.ts';
import { Entity, EntityData } from './Entity.ts';

interface TeamData extends EntityData {
teamId: string;
orgId: string;
name: string;
description: string;
}

export type CreateTeamData = Omit<EasyData<TeamData, 'teamId'>, 'id' | 'description'> & { description?: string };

export class Team extends Entity {
teamId: string;
orgId: string;
name: string;
description: string;

constructor(data: TeamData) {
super(data);
this.teamId = data.teamId;
this.orgId = data.orgId;
this.name = data.name;
this.description = data.description ?? '';
}

static create(data: CreateTeamData): Team {
const fullData = { ...data, description: data.description ?? '' };
const newData = EntityUtil.defaultData(fullData, 'teamId');
return new Team(newData);
}
}
28 changes: 28 additions & 0 deletions app/core/entity/TeamMember.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { EasyData, EntityUtil } from '../util/EntityUtil.ts';
import { Entity, EntityData } from './Entity.ts';

interface TeamMemberData extends EntityData {
teamMemberId: string;
teamId: string;
userId: string;
}

export type CreateTeamMemberData = Omit<EasyData<TeamMemberData, 'teamMemberId'>, 'id'>;

export class TeamMember extends Entity {
teamMemberId: string;
teamId: string;
userId: string;

constructor(data: TeamMemberData) {
super(data);
this.teamMemberId = data.teamMemberId;
this.teamId = data.teamId;
this.userId = data.userId;
}

static create(data: CreateTeamMemberData): TeamMember {
const newData = EntityUtil.defaultData(data, 'teamMemberId');
return new TeamMember(newData);
}
}
28 changes: 28 additions & 0 deletions app/core/entity/TeamPackage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { EasyData, EntityUtil } from '../util/EntityUtil.ts';
import { Entity, EntityData } from './Entity.ts';

interface TeamPackageData extends EntityData {
teamPackageId: string;
teamId: string;
packageId: string;
}

export type CreateTeamPackageData = Omit<EasyData<TeamPackageData, 'teamPackageId'>, 'id'>;

export class TeamPackage extends Entity {
teamPackageId: string;
teamId: string;
packageId: string;

constructor(data: TeamPackageData) {
super(data);
this.teamPackageId = data.teamPackageId;
this.teamId = data.teamId;
this.packageId = data.packageId;
}

static create(data: CreateTeamPackageData): TeamPackage {
const newData = EntityUtil.defaultData(data, 'teamPackageId');
return new TeamPackage(newData);
}
}
141 changes: 141 additions & 0 deletions app/core/service/OrgService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import { AccessLevel, Inject, SingletonProto } from 'egg';
import { ForbiddenError, NotFoundError } from 'egg/errors';

import { AbstractService } from '../../common/AbstractService.ts';
import { DEVELOPERS_TEAM } from '../../common/constants.ts';
import type { OrgRepository } from '../../repository/OrgRepository.ts';
import type { TeamRepository } from '../../repository/TeamRepository.ts';
import { Org } from '../entity/Org.ts';
import { OrgMember } from '../entity/OrgMember.ts';
import { Team } from '../entity/Team.ts';
import { TeamMember } from '../entity/TeamMember.ts';

export interface CreateOrgCmd {
name: string;
description?: string;
creatorUserId: string;
}

@SingletonProto({
accessLevel: AccessLevel.PUBLIC,
})
export class OrgService extends AbstractService {
@Inject()
private readonly orgRepository: OrgRepository;

@Inject()
private readonly teamRepository: TeamRepository;

async createOrg(cmd: CreateOrgCmd): Promise<Org> {
const existing = await this.orgRepository.findOrgByName(cmd.name);
if (existing) {
throw new ForbiddenError(`Org "${cmd.name}" already exists`);
}

// Create org + developers team + owner + team member in one transaction
const org = Org.create({
name: cmd.name,
description: cmd.description,
});
const developersTeam = Team.create({
orgId: org.orgId,
name: DEVELOPERS_TEAM,
description: 'default team',
});
const ownerMember = OrgMember.create({
orgId: org.orgId,
userId: cmd.creatorUserId,
role: 'owner',
});
const teamMember = TeamMember.create({
teamId: developersTeam.teamId,
userId: cmd.creatorUserId,
});
await this.orgRepository.createOrgCascade(org, developersTeam, ownerMember, teamMember);
Comment on lines +29 to +54
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid the check-then-insert race in org creation.

Both createOrg() and ensureOrgForScope() do findOrgByName() before inserting. Concurrent requests for the same name can pass that existence check together and then collide in createOrgCascade() / saveOrg(), which means either duplicate org rows or a raw DB uniqueness error escaping to the caller. Please make org creation idempotent at the repository/DB boundary and re-read on conflict.

Also applies to: 75-85

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/core/service/OrgService.ts` around lines 29 - 54, The
createOrg/ensureOrgForScope flow is vulnerable to a check-then-insert race
because both call findOrgByName() before inserting; fix this at the
repository/DB boundary by making createOrgCascade/saveOrg idempotent and
retry/read on conflict: when createOrgCascade or saveOrg triggers a
unique-constraint/duplicate-key error, catch that specific DB error, re-query
findOrgByName() and return the existing Org instead of letting the raw error
bubble up (or implement an upsert that returns the existing row); update
orgRepository.createOrgCascade and any saveOrg/upsert helpers to perform this
conflict handling so createOrg and ensureOrgForScope no longer rely solely on
the pre-check.


this.logger.info(
'[OrgService:createOrg] orgId: %s, name: %s, creatorUserId: %s',
org.orgId,
org.name,
cmd.creatorUserId,
);
return org;
}

async removeOrg(orgId: string): Promise<void> {
await this.orgRepository.removeOrgCascade(orgId);
this.logger.info('[OrgService:removeOrg] orgId: %s', orgId);
}

async findOrgByName(name: string): Promise<Org | null> {
return await this.orgRepository.findOrgByName(name);
}

// Auto-create org for allowScopes if it doesn't exist
async ensureOrgForScope(scope: string): Promise<Org> {
const orgName = scope.replace(/^@/, '');
const existing = await this.orgRepository.findOrgByName(orgName);
if (existing) return existing;

const org = Org.create({
name: orgName,
description: `Auto-created org for scope ${scope}`,
});
await this.orgRepository.saveOrg(org);
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ensureOrgForScope() creates an Org record but does not create the default developers team. This is inconsistent with createOrg() and with npm’s expectation that @scope:developers exists, and it will break flows like npm access grant ... @scope:developers ... on a newly auto-created allowScopes org. Consider creating the developers team (and any required defaults) as part of the auto-create path (ideally transactionally), or make the absence explicit and handle it in TeamController/TeamService.

Suggested change
await this.orgRepository.saveOrg(org);
await this.orgRepository.saveOrg(org);
// Ensure default developers team exists for this org, matching createOrg() behavior
const developersTeam = Team.create({
orgId: org.orgId,
name: DEVELOPERS_TEAM,
description: 'Default developers team',
});
await this.teamRepository.saveTeam(developersTeam);

Copilot uses AI. Check for mistakes.
this.logger.info('[OrgService:ensureOrgForScope] orgId: %s, scope: %s', org.orgId, scope);
return org;
}

async addMember(orgId: string, userId: string, role: 'owner' | 'member' = 'member'): Promise<OrgMember> {
const org = await this.orgRepository.findOrgByOrgId(orgId);
if (!org) {
throw new NotFoundError('Org not found');
}

// Upsert org member
let member = await this.orgRepository.findMember(orgId, userId);
if (member) {
member.role = role;
await this.orgRepository.saveMember(member);
} else {
member = OrgMember.create({ orgId, userId, role });
await this.orgRepository.saveMember(member);
}

// Auto-add to developers team
const developersTeam = await this.teamRepository.findTeam(orgId, DEVELOPERS_TEAM);
if (developersTeam) {
const existingTeamMember = await this.teamRepository.findMember(developersTeam.teamId, userId);
if (!existingTeamMember) {
const teamMember = TeamMember.create({
teamId: developersTeam.teamId,
userId,
});
await this.teamRepository.addMember(teamMember);
}
}

this.logger.info('[OrgService:addMember] orgId: %s, userId: %s, role: %s', orgId, userId, role);
return member;
}

async removeMember(orgId: string, userId: string): Promise<void> {
// Remove from all teams in this org
await this.teamRepository.removeMemberFromAllTeams(orgId, userId);
// Remove from org
await this.orgRepository.removeMember(orgId, userId);
this.logger.info('[OrgService:removeMember] orgId: %s, userId: %s', orgId, userId);
}

async listMembers(orgId: string): Promise<OrgMember[]> {
return await this.orgRepository.listMembers(orgId);
}

async requiredOrgOwnerOrAdmin(orgId: string, userId: string, isAdmin: boolean): Promise<void> {
if (isAdmin) return;
const member = await this.orgRepository.findMember(orgId, userId);
if (!member || member.role !== 'owner') {
throw new ForbiddenError('Only org owner or admin can perform this action');
}
}
}
Loading
Loading