fix: sanitize get user response (#3390)
Maintain Release Merge PR / update-release-pr (push) Has been cancelled
Notify HeyPuter / notify (push) Has been cancelled
release-please / release-please (push) Has been cancelled

* fix: sanitize get user response

* fix: app creation in dev center
This commit is contained in:
Daniel Salazar
2026-07-15 02:02:06 -07:00
committed by GitHub
parent 92a6d30e26
commit cf34f9fea9
7 changed files with 229 additions and 17 deletions
+43
View File
@@ -158,6 +158,49 @@ describe('whoami extension — handleWhoami', () => {
expect(body.directories).toBeUndefined();
});
it('redacts tmp_password from metadata for user actors', async () => {
const user = await seedUser();
await server.stores.user.updateMetadata(user.id as number, {
tmp_password: 'bootstrap-secret',
hasDevAccountAccess: true,
});
const { res, captured } = makeRes();
await runWithContext(
{ actor: { user: { uuid: user.uuid, id: user.id as number } } },
() => handleWhoami(makeReq(), res),
);
const body = captured.body as Record<string, unknown>;
const metadata = body.metadata as Record<string, unknown>;
expect(metadata.tmp_password).toBeUndefined();
// Other metadata keys still reach the user's own client.
expect(metadata.hasDevAccountAccess).toBe(true);
expect(body.hasDevAccountAccess).toBe(true);
});
it('never sends user metadata to app actors', async () => {
const user = await seedUser();
await server.stores.user.updateMetadata(user.id as number, {
tmp_password: 'bootstrap-secret',
});
const { res, captured } = makeRes();
await runWithContext(
{
actor: {
user: { uuid: user.uuid, id: user.id as number },
app: { uid: 'app-test-actor' },
},
},
() => handleWhoami(makeReq(), res),
);
const body = captured.body as Record<string, unknown>;
expect(body.metadata).toBeUndefined();
expect(JSON.stringify(body)).not.toContain('bootstrap-secret');
});
it('marks the user as oidc_only when password is null', async () => {
const slug = Math.random().toString(36).slice(2, 8);
const oidcUser = await server.stores.user.create({
+5 -1
View File
@@ -65,6 +65,9 @@ export const handleWhoami = async (
}
}
const metadata = user.metadata ? { ...user.metadata } : user.metadata;
if (metadata) delete metadata.tmp_password;
const details: Record<string, unknown> = {
username: user.username,
uuid: user.uuid,
@@ -98,7 +101,7 @@ export const handleWhoami = async (
human_readable_age: user.timestamp
? timeago.format(new Date(user.timestamp as string))
: null,
metadata: user.metadata,
metadata,
hasDevAccountAccess: !!user.metadata?.hasDevAccountAccess,
};
@@ -162,6 +165,7 @@ export const handleWhoami = async (
delete details.desktop_bg_fit;
delete details.human_readable_age;
delete details.is_user_token;
delete details.metadata;
}
if (actor.app) {
+9 -1
View File
@@ -1117,7 +1117,15 @@ export class AppDriver extends PuterDriver {
const subdomain = this.#extractPuterHostedSubdomain(indexUrl);
if (!subdomain) return;
const row = await this.stores.subdomain.getBySubdomain(subdomain);
let row = await this.stores.subdomain.getBySubdomain(subdomain);
if (!row) {
// Deploys create the subdomain and immediately point the app
// at it, so a replica or peer-cache miss here would wrongly
// refuse the owner. Confirm against the primary before failing.
row = await this.stores.subdomain.getBySubdomain(subdomain, {
primary: true,
});
}
if (!row || row.user_id !== user.id) {
throw new HttpError(400, 'Subdomain not owned by user', {
legacyCode: 'subdomain_not_owned',
@@ -1081,3 +1081,98 @@ describe('AppDriver alias-group index_url merge', () => {
expect(first.uid).not.toBe(second.uid);
});
});
// ── hosted-subdomain ownership check ────────────────────────────────
//
// `#ensurePuterSiteSubdomainIsOwned` gates puter-hosted index_urls on a
// subdomain row the caller owns. Deploy flows create that row and point
// the app at it in back-to-back requests, so the check must tolerate a
// replica/cache miss by confirming against the primary before refusing.
describe('AppDriver hosted-subdomain ownership check', () => {
const hostedUrl = (sub: string) => `https://${sub}.site.puter.localhost/`;
it('accepts a hosted index_url when the subdomain row has not reached the replica yet', async () => {
const { actor, userId } = await makeUser();
const sub = uniqueName('deploy');
await server.stores.subdomain.create({ userId, subdomain: sub });
// Simulate a peer node with a lagging replica: no cache entry for
// the row, and replica reads (`read`) that don't see it yet while
// primary reads (`pread`) do. Sqlite's `pread` delegates to
// `this.read`, so pin it to the original before stubbing `read`.
await server.clients.redis.del(`subdomains:name:${sub}`);
const db = server.clients.db as unknown as {
read: (q: string, p?: unknown[]) => Promise<unknown[]>;
pread: (q: string, p?: unknown[]) => Promise<unknown[]>;
};
const originalRead = db.read.bind(server.clients.db);
const hadOwnPread = Object.prototype.hasOwnProperty.call(db, 'pread');
db.pread = async (query: string, params?: unknown[]) =>
originalRead(query, params);
db.read = async (query: string, params?: unknown[]) =>
query.includes('FROM `subdomains`') && (params ?? []).includes(sub)
? []
: originalRead(query, params);
try {
const result = await withActor(actor, () =>
driver.create({
object: {
name: uniqueName('app'),
title: 'Deployed App',
index_url: hostedUrl(sub),
},
}),
);
expect(result.uid).toEqual(expect.any(String));
// The primary hit must also heal the stale cache: a normal
// lookup now resolves from cache even though the replica
// still misses.
const healed =
await server.stores.subdomain.getBySubdomain(sub);
expect(healed?.subdomain).toBe(sub);
} finally {
delete (db as { read?: unknown }).read;
if (!hadOwnPread) delete (db as { pread?: unknown }).pread;
}
});
it('rejects a hosted index_url whose subdomain does not exist anywhere', async () => {
const { actor } = await makeUser();
await expect(
withActor(actor, () =>
driver.create({
object: {
name: uniqueName('app'),
title: 'x',
index_url: hostedUrl(uniqueName('ghost')),
},
}),
),
).rejects.toMatchObject({ statusCode: 400 });
});
it("rejects a hosted index_url pointing at another user's subdomain", async () => {
const owner = await makeUser();
const intruder = await makeUser();
const sub = uniqueName('theirs');
await server.stores.subdomain.create({
userId: owner.userId,
subdomain: sub,
});
await expect(
withActor(intruder.actor, () =>
driver.create({
object: {
name: uniqueName('app'),
title: 'x',
index_url: hostedUrl(sub),
},
}),
),
).rejects.toMatchObject({ statusCode: 400 });
});
});
@@ -0,0 +1,43 @@
import bcrypt from 'bcrypt';
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
import { PuterServer } from '../../server.ts';
import { setupTestServer } from '../../testUtil.ts';
let server: PuterServer;
beforeAll(async () => {
server = await setupTestServer({
no_default_user: false,
} as never);
});
afterAll(async () => {
await server.shutdown();
});
describe('DefaultUserService — bootstrap admin credentials', () => {
it('stashes the bootstrap password so rotation can be detected', async () => {
const admin = await server.stores.user.getByUsername('admin');
expect(admin).toBeTruthy();
const stashed = admin?.metadata?.tmp_password;
expect(typeof stashed).toBe('string');
expect(
await bcrypt.compare(String(stashed), String(admin?.password)),
).toBe(true);
});
it('drops the plaintext stash once the password is rotated', async () => {
const admin = await server.stores.user.getByUsername('admin');
expect(admin).toBeTruthy();
await server.stores.user.update(admin!.id, {
password: await bcrypt.hash('rotated-password', 8),
});
// Simulate the next boot.
await server.services.defaultUser.onServerStart();
const fresh = await server.stores.user.getByUsername('admin');
expect(fresh?.metadata?.tmp_password ?? null).toBeNull();
});
});
@@ -41,7 +41,8 @@ const ADMIN_STORAGE_BYTES = 10 * 1024 * 1024 * 1024;
*
* Each boot where the current password hash still matches the stashed
* plaintext, the credentials are re-printed to stdout (CI scrapes this
* line to extract the default password).
* line to extract the default password). Once rotation is detected the
* stash is deleted so the plaintext isn't retained longer than needed.
*/
export class DefaultUserService extends PuterService {
override async onServerStart(): Promise<void> {
@@ -88,7 +89,13 @@ export class DefaultUserService extends PuterService {
tmpPassword,
String(user.password),
);
if (!isDefault) return;
if (!isDefault) {
// Password was rotated — stop retaining the plaintext stash.
await this.stores.user.updateMetadata(user.id, {
tmp_password: null,
});
return;
}
this.#printCredentials(tmpPassword);
}
+25 -13
View File
@@ -107,25 +107,37 @@ export class SubdomainStore extends PuterStore {
return (rows[0] as unknown as SubdomainRow) ?? null;
}
async getBySubdomain(subdomain: string): Promise<SubdomainRow | null> {
/**
* Pass `primary: true` for read-after-write lookups (e.g. checking a
* subdomain that may have been created moments ago): it skips the
* cache which may hold a stale negative marker and reads the
* primary instead of a possibly-lagging replica. The result still
* refreshes the cache, healing any stale marker.
*/
async getBySubdomain(
subdomain: string,
{ primary = false }: { primary?: boolean } = {},
): Promise<SubdomainRow | null> {
if (!subdomain) return null;
const cacheKey = this.#cacheKey(subdomain);
try {
const raw = await this.clients.redis.get(cacheKey);
if (raw === NEGATIVE_CACHE_MARKER) return null;
if (raw) {
const parsed = JSON.parse(raw) as SubdomainRow | null;
if (parsed) return parsed;
if (!primary) {
try {
const raw = await this.clients.redis.get(cacheKey);
if (raw === NEGATIVE_CACHE_MARKER) return null;
if (raw) {
const parsed = JSON.parse(raw) as SubdomainRow | null;
if (parsed) return parsed;
}
} catch {
/* fall through */
}
} catch {
/* fall through */
}
const rows = await this.clients.db.read(
'SELECT * FROM `subdomains` WHERE `subdomain` = ? LIMIT 1',
[subdomain],
);
const sql = 'SELECT * FROM `subdomains` WHERE `subdomain` = ? LIMIT 1';
const rows = primary
? await this.clients.db.pread(sql, [subdomain])
: await this.clients.db.read(sql, [subdomain]);
const row = (rows[0] as unknown as SubdomainRow | undefined) ?? null;
if (row) {