also looking at this
fix(security): toArray() now applies hidden, and toJSON() delegates to it
#1184
2 files
+126
-24
| @@ -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 |
| @@ -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)) { |
| @@ -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 | } |