fix(security): toArray() now applies hidden, and toJSON() delegates to it

packages/bun-query-builder/src/orm.tsmodified+38-24

Changes to packages/bun-query-builder/src/orm.ts

		⋯1,233 linesShow the 1,233 lines above this hunk@@ -1234,6 +1234,11 @@ class ModelInstance<
1234	1234	* Plain-object snapshot of all attribute values. Method form so callers
1235	1235	* can write `instance.getAttributes()` (Eloquent-style); `instance.attributes`
1236	1236	* remains as a getter. Both return a shallow copy.
	1237	+ *
	1238	+ * **`hidden: true` is not applied here.** This is the raw accessor, and
	1239	+ * dirty-tracking and diffing need the whole row. Use `toArray()` or
	1240	+ * `toJSON()` for anything that leaves the process - both redact. Deliberate,
	1241	+ * unlike the `toArray()` leak in stacksjs/bun-query-builder#1182.
1237	1242	*/
1238	1243	getAttributes(): Pick<ModelAttributes<TDef>, TSelected & keyof ModelAttributes<TDef>> {
1239	1244	return { ...this._attributes } as any
		⋯477 linesShow the 477 lines above this hunk@@ -1717,8 +1722,33 @@ class ModelInstance<
1717	1722	return new ModelInstance<TDef, TSelected>(this._definition, attrs as any, false)
1718	1723	}
1719	1724
	1725	+ /**
	1726	+ * The model as a plain object, with `hidden: true` attributes removed.
	1727	+ *
	1728	+ * It used to spread `_attributes` whole, so a `password` or `api_token`
	1729	+ * marked hidden came out in full while `toJSON()`, written directly beneath
	1730	+ * it, redacted the same field. Two serializers side by side with opposite
	1731	+ * behaviour is the trap: this API takes its names from Eloquent, where
	1732	+ * `toArray()` applies `$hidden` and `toJSON()` is defined in terms of it, so
	1733	+ * the expectation someone arrives with is the wrong one. Relations are
	1734	+ * serialized through `toArray()` too, which propagated the leak into every
	1735	+ * eager-loaded payload - the same shape as #1093.
	1736	+ *
	1737	+ * `toJSON()` now delegates here, so the two cannot drift apart again.
	1738	+ * stacksjs/bun-query-builder#1182
	1739	+ */
1720	1740	toArray(): Record<string, unknown> {
1721		- const values: Record<string, unknown> = { ...this._attributes }
	1741	+ const hidden = new Set<string>()
	1742	+ for (const [key, attr] of Object.entries(this._definition.attributes)) {
	1743	+ if (attr.hidden)
	1744	+ hidden.add(key)
	1745	+ }
	1746	+
	1747	+ const values: Record<string, unknown> = {}
	1748	+ for (const [key, value] of Object.entries(this._attributes)) {
	1749	+ if (!hidden.has(key))
	1750	+ values[key] = value
	1751	+ }
1722	1752
1723	1753	for (const [relName, relData] of Object.entries(this._relations)) {
1724	1754	if (Array.isArray(relData)) {
		⋯10 linesShow the 10 lines above this hunk@@ -1735,30 +1765,14 @@ class ModelInstance<
1735	1765	return values
1736	1766	}
1737	1767
	1768	+ /**
	1769	+ * Delegates to {@link toArray}, which applies the same `hidden` filter and
	1770	+ * serializes relations the same way. Kept as one implementation so the two
	1771	+ * cannot diverge again - they already had, and only `toJSON()` was redacting.
	1772	+ * stacksjs/bun-query-builder#1182
	1773	+ */
1738	1774	toJSON(): Omit<Pick<ModelAttributes<TDef>, TSelected & keyof ModelAttributes<TDef>>, HiddenKeys<TDef>> {
1739		- const hidden = new Set<string>()
1740		- for (const [key, attr] of Object.entries(this._definition.attributes)) {
1741		- if (attr.hidden) hidden.add(key)
1742		- }
1743		-
1744		- const json: Record<string, unknown> = {}
1745		- for (const [key, value] of Object.entries(this._attributes)) {
1746		- if (!hidden.has(key)) json[key] = value
1747		- }
1748		-
1749		- for (const [relName, relData] of Object.entries(this._relations)) {
1750		- if (Array.isArray(relData)) {
1751		- json[relName] = relData.map(r => r.toJSON())
1752		- }
1753		- else if (relData) {
1754		- json[relName] = relData.toJSON()
1755		- }
1756		- else {
1757		- json[relName] = null
1758		- }
1759		- }
1760		-
1761		- return json as any
	1775	+ return this.toArray() as any
1762	1776	}
1763	1777
1764	1778	}

packages/bun-query-builder/test/orm.hidden-attributes.test.tsadded+88-0

Changes to packages/bun-query-builder/test/orm.hidden-attributes.test.ts

		@@ -0,0 +1,88 @@
	1	+/**
	2	+ * `hidden: true` across the serialization boundary —
	3	+ * stacksjs/bun-query-builder#1182.
	4	+ *
	5	+ * `toJSON()` redacted and `toArray()`, written directly beneath it, did not:
	6	+ *
	7	+ * toJSON() {"id":1,"email":"a@b.c"}
	8	+ * toArray() {"id":1,"email":"a@b.c","password":"bcrypt$hash","api_token":"sk_live_abc"}
	9	+ *
	10	+ * Two serializers side by side with opposite behaviour. This API takes its
	11	+ * names from Eloquent, where `toArray()` applies `$hidden` and `toJSON()` is
	12	+ * defined in terms of it, so the expectation someone arrives with is the wrong
	13	+ * one — they reach for `toArray()` in a response serializer and ship the hash.
	14	+ *
	15	+ * `toJSON()` now delegates to `toArray()`, so the two cannot drift again.
	16	+ */
	17	+
	18	+import { describe, expect, it } from 'bun:test'
	19	+import { createModel } from '../src/orm'
	20	+
	21	+function user(): any {
	22	+ const User: any = createModel({
	23	+ name: 'User',
	24	+ table: 'users',
	25	+ primaryKey: 'id',
	26	+ attributes: {
	27	+ id: { validation: { rule: {} } },
	28	+ email: { validation: { rule: {} } },
	29	+ password: { validation: { rule: {} }, hidden: true },
	30	+ api_token: { validation: { rule: {} }, hidden: true },
	31	+ },
	32	+ } as any)
	33	+ return User.make({ id: 1, email: 'a@b.c', password: 'bcrypt$hash', api_token: 'sk_live_abc' })
	34	+}
	35	+
	36	+describe('hidden attributes are redacted when serialized (#1182)', () => {
	37	+ it('toArray() drops them, which is the regression', () => {
	38	+ const out = user().toArray()
	39	+ expect(out.password).toBeUndefined()
	40	+ expect(out.api_token).toBeUndefined()
	41	+ // Still a usable snapshot of everything else.
	42	+ expect(out.id).toBe(1)
	43	+ expect(out.email).toBe('a@b.c')
	44	+ })
	45	+
	46	+ it('toJSON() drops them too, and agrees with toArray()', () => {
	47	+ const u = user()
	48	+ expect(JSON.stringify(u.toJSON())).toBe(JSON.stringify(u.toArray()))
	49	+ })
	50	+
	51	+ it('JSON.stringify of the model drops them', () => {
	52	+ const encoded = JSON.stringify(user())
	53	+ expect(encoded).not.toContain('bcrypt')
	54	+ expect(encoded).not.toContain('sk_live')
	55	+ })
	56	+
	57	+ it('neither serializer leaks the secret as a substring', () => {
	58	+ // Blunt, but it is the property that actually matters at a response
	59	+ // boundary: the value must not appear anywhere in the payload.
	60	+ for (const payload of [JSON.stringify(user().toArray()), JSON.stringify(user().toJSON())]) {
	61	+ expect(payload).not.toContain('bcrypt$hash')
	62	+ expect(payload).not.toContain('sk_live_abc')
	63	+ }
	64	+ })
	65	+
	66	+ it('getAttributes() still returns everything, deliberately', () => {
	67	+ // The raw accessor: dirty-tracking and diffing need the whole row. It is
	68	+ // documented as unredacted, which is the difference between this and the
	69	+ // toArray() bug.
	70	+ const attrs = user().getAttributes()
	71	+ expect(attrs.password).toBe('bcrypt$hash')
	72	+ })
	73	+})
	74	+
	75	+describe('redaction reaches eager-loaded relations (#1182)', () => {
	76	+ it('a related model is serialized through the same filter', () => {
	77	+ // toArray() recurses with toArray(), so before the fix an eager-loaded
	78	+ // relation carried the related model's secrets into the parent payload —
	79	+ // the same propagation shape as #1093.
	80	+ const parent = user()
	81	+ parent._relations = { author: user(), readers: [user(), user()] }
	82	+
	83	+ const out: any = parent.toArray()
	84	+ expect(out.author.password).toBeUndefined()
	85	+ expect(out.readers[0].password).toBeUndefined()
	86	+ expect(JSON.stringify(out)).not.toContain('bcrypt$hash')
	87	+ })
	88	+})