fix(auth): stop leaking usernames via emailDomain span attribute
getUserByUsername emitted the FULL username as the emailDomain span attribute whenever the username contained no "@" (audit finding B6), violating the PII-free telemetry rule (ADR-017 §7). Emit only a boolean hasAtSign in both the production repository and its mock; regression tests pin that no username-derived string reaches span attributes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -48,7 +48,9 @@ export class MockUsersRepository implements IUsersRepository {
|
|||||||
{
|
{
|
||||||
name: "users.getUserByUsername",
|
name: "users.getUserByUsername",
|
||||||
op: "repository",
|
op: "repository",
|
||||||
attributes: { emailDomain: username.includes("@") ? (username.split("@")[1] ?? "(invalid)") : username },
|
// Never emit the username (or any slice of it) — it is PII and the
|
||||||
|
// non-email branch used to leak the full username (audit finding B6).
|
||||||
|
attributes: { hasAtSign: username.includes("@") },
|
||||||
},
|
},
|
||||||
async (span) => {
|
async (span) => {
|
||||||
const found = this._users.find((u) => u.username === username);
|
const found = this._users.find((u) => u.username === username);
|
||||||
@@ -60,7 +62,11 @@ export class MockUsersRepository implements IUsersRepository {
|
|||||||
|
|
||||||
async createUser(input: User): Promise<User> {
|
async createUser(input: User): Promise<User> {
|
||||||
return this.tracer.startSpan(
|
return this.tracer.startSpan(
|
||||||
{ name: "users.createUser", op: "repository", attributes: { id: input.id } },
|
{
|
||||||
|
name: "users.createUser",
|
||||||
|
op: "repository",
|
||||||
|
attributes: { id: input.id },
|
||||||
|
},
|
||||||
async (span) => {
|
async (span) => {
|
||||||
this._users.push(input);
|
this._users.push(input);
|
||||||
span.setAttribute("created", true);
|
span.setAttribute("created", true);
|
||||||
|
|||||||
@@ -21,17 +21,31 @@ describe("MockUsersRepository emits spans", () => {
|
|||||||
expect(tracer.spans[0]!.attributes.found).toBe(false);
|
expect(tracer.spans[0]!.attributes.found).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("getUserByUsername emits a span with emailDomain attribute", async () => {
|
it("getUserByUsername emits a span without any username-derived PII", async () => {
|
||||||
const tracer = new RecordingTracer();
|
const tracer = new RecordingTracer();
|
||||||
const repo = new MockUsersRepository(
|
const repo = new MockUsersRepository(
|
||||||
[{ id: "1", username: "alice", passwordHash: "hash" }],
|
[{ id: "1", username: "alice", passwordHash: "hash" }],
|
||||||
tracer,
|
tracer,
|
||||||
);
|
);
|
||||||
await repo.getUserByUsername("alice");
|
await repo.getUserByUsername("alice");
|
||||||
expect(tracer.findSpan("users.getUserByUsername")).toBeDefined();
|
const span = tracer.findSpan("users.getUserByUsername");
|
||||||
expect(tracer.findSpan("users.getUserByUsername")!.attributes.found).toBe(
|
expect(span).toBeDefined();
|
||||||
true,
|
expect(span!.attributes.found).toBe(true);
|
||||||
);
|
// B6 regression guard: the old emailDomain attribute leaked the full
|
||||||
|
// username when it contained no "@". Only a boolean may be emitted.
|
||||||
|
expect(span!.attributes.emailDomain).toBeUndefined();
|
||||||
|
expect(span!.attributes.hasAtSign).toBe(false);
|
||||||
|
expect(Object.values(span!.attributes)).not.toContain("alice");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("getUserByUsername with an email-shaped username emits only the boolean", async () => {
|
||||||
|
const tracer = new RecordingTracer();
|
||||||
|
const repo = new MockUsersRepository([], tracer);
|
||||||
|
await repo.getUserByUsername("alice@example.com");
|
||||||
|
const span = tracer.findSpan("users.getUserByUsername");
|
||||||
|
expect(span!.attributes.hasAtSign).toBe(true);
|
||||||
|
expect(span!.attributes.emailDomain).toBeUndefined();
|
||||||
|
expect(Object.values(span!.attributes)).not.toContain("example.com");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("createUser records created=true", async () => {
|
it("createUser records created=true", async () => {
|
||||||
|
|||||||
@@ -79,11 +79,9 @@ export class UsersRepository implements IUsersRepository {
|
|||||||
{
|
{
|
||||||
name: "users.getUserByUsername",
|
name: "users.getUserByUsername",
|
||||||
op: "repository",
|
op: "repository",
|
||||||
attributes: {
|
// Never emit the username (or any slice of it) — it is PII and the
|
||||||
emailDomain: username.includes("@")
|
// non-email branch used to leak the full username (audit finding B6).
|
||||||
? (username.split("@")[1] ?? "(invalid)")
|
attributes: { hasAtSign: username.includes("@") },
|
||||||
: username,
|
|
||||||
},
|
|
||||||
},
|
},
|
||||||
async (span) => {
|
async (span) => {
|
||||||
try {
|
try {
|
||||||
|
|||||||
Reference in New Issue
Block a user