Testing Migrations

server/tests/helpers/Schema.test.js

@@ -0,0 +1,243 @@
+require('module-alias/register')
+
+const Schema = require('@helpers/Schema')
+const db = require('@config/database')
+
+afterAll(() => {
+	db.end()
+})
+
+afterEach(() => {
+	Schema.__reset()
+})
+
+describe('verifyScrapeSchema against the real dev schema', () => {
+
+	test('reports ok once the migration has been applied', async () => {
+		const result = await Schema.verifyScrapeSchema()
+
+		if (!result.ok) {
+			throw new Error(
+				`dev scrape schema not applied - run ./migrateDatabase.sh. ` +
+				`missing objects: ${result.missingObjects.join(', ') || 'none'}; ` +
+				`missing privileges: ${result.missingPrivileges.join(', ') || 'none'}`
+			)
+		}
+
+		expect(result.missingObjects).toEqual([])
+		expect(result.missingPrivileges).toEqual([])
+		expect(result.remedy).toBe('')
+	})
+
+	test('isScrapeSchemaReady reflects the last verification', async () => {
+		expect(Schema.isScrapeSchemaReady()).toBe(false)
+
+		await Schema.verifyScrapeSchema()
+		expect(Schema.isScrapeSchemaReady()).toBe(true)
+
+		Schema.__reset()
+		expect(Schema.isScrapeSchemaReady()).toBe(false)
+	})
+
+	test('__setReady is honoured, so request paths can be driven in tests', () => {
+		Schema.__setReady(true)
+		expect(Schema.isScrapeSchemaReady()).toBe(true)
+		Schema.__setReady(false)
+		expect(Schema.isScrapeSchemaReady()).toBe(false)
+	})
+})
+
+//
+// The FULLTEXT index is the one object whose loss is silent: MATCH..AGAINST
+// against a BTREE index returns nothing without erroring. Guard it by type.
+//
+describe('FULLTEXT index guard', () => {
+
+	test('attachment_text_index exists and is actually FULLTEXT', async () => {
+		await expect(Schema.__fulltextIndexExists('attachment', 'attachment_text_index'))
+			.resolves.toBe(true)
+	})
+
+	test('a BTREE index of the same shape would NOT satisfy the check', async () => {
+		// attachment_scrape_state is a real BTREE index, so this proves the
+		// check discriminates on type rather than just on name existing.
+		await expect(Schema.__indexExists('attachment', 'attachment_scrape_state'))
+			.resolves.toBe(true)
+		await expect(Schema.__fulltextIndexExists('attachment', 'attachment_scrape_state'))
+			.resolves.toBe(false)
+	})
+
+	test('verifyScrapeSchema includes it among the verified objects', async () => {
+		const result = await Schema.verifyScrapeSchema()
+		expect(result.ok).toBe(true)
+		expect(result.missingObjects).toEqual([])
+	})
+
+	test('MATCH..AGAINST actually works, not just that an index exists', async () => {
+		// The functional end of the guard. An index of the right type that
+		// somehow could not be searched would still be a broken search.
+		const [rows] = await db.promise().query(
+			`SELECT COUNT(*) AS n FROM attachment
+			 WHERE MATCH(text) AGAINST(? IN NATURAL LANGUAGE MODE)`,
+			['the']
+		)
+		// Any result (including 0 matches) proves the query planner accepted
+		// MATCH..AGAINST against this column, which a BTREE index would reject.
+		expect(typeof rows[0].n).toBe('number')
+	})
+})
+
+describe('object existence checks', () => {
+
+	test('finds the real table, columns and index', async () => {
+		await expect(Schema.__tableExists('scrape_cache')).resolves.toBe(true)
+		await expect(Schema.__columnExists('attachment', 'scrape_state')).resolves.toBe(true)
+		await expect(Schema.__columnExists('attachment', 'scrape_attempts')).resolves.toBe(true)
+		await expect(Schema.__columnExists('attachment', 'enqueued_at')).resolves.toBe(true)
+		await expect(Schema.__indexExists('attachment', 'attachment_scrape_state')).resolves.toBe(true)
+	})
+
+	test('reports absence rather than throwing for things that do not exist', async () => {
+		await expect(Schema.__tableExists('no_such_table_xyz')).resolves.toBe(false)
+		await expect(Schema.__columnExists('attachment', 'no_such_column_xyz')).resolves.toBe(false)
+		await expect(Schema.__indexExists('attachment', 'no_such_index_xyz')).resolves.toBe(false)
+	})
+})
+
+//
+// The privilege check is what turns a future narrow-grants problem into one
+// clear boot message instead of a swallowed runtime failure.
+//
+describe('parseGrants', () => {
+
+	const DB = 'application'
+	const TABLES = ['attachment', 'scrape_cache']
+	const parse = (lines) => Schema.parseGrants(lines, DB, TABLES)
+
+	test('ALL PRIVILEGES ON *.* grants everything', () => {
+		const granted = parse(['GRANT ALL PRIVILEGES ON *.* TO `root`@`localhost` WITH GRANT OPTION'])
+
+		expect(granted.has('SELECT')).toBe(true)
+		expect(granted.has('INSERT')).toBe(true)
+		expect(granted.has('UPDATE')).toBe(true)
+		expect(granted.has('DELETE')).toBe(true)
+	})
+
+	test('ALL PRIVILEGES scoped to the database grants everything', () => {
+		const granted = parse(['GRANT ALL PRIVILEGES ON `application`.* TO `app`@`localhost`'])
+		expect(granted.has('DELETE')).toBe(true)
+	})
+
+	test('reads an explicit privilege list', () => {
+		const granted = parse([
+			'GRANT USAGE ON *.* TO `app`@`localhost`',
+			'GRANT SELECT, INSERT, UPDATE, DELETE ON `application`.* TO `app`@`localhost`',
+		])
+
+		expect([...granted].sort()).toEqual(['DELETE', 'INSERT', 'SELECT', 'UPDATE'])
+	})
+
+	// The case that caused this whole change class.
+	test('spots a missing DELETE', () => {
+		const granted = parse(['GRANT SELECT, INSERT, UPDATE ON `application`.* TO `app`@`localhost`'])
+
+		expect(granted.has('UPDATE')).toBe(true)
+		expect(granted.has('DELETE')).toBe(false)
+	})
+
+	test('accepts table-scoped grants for our tables', () => {
+		const granted = parse([
+			'GRANT SELECT, DELETE ON `application`.`attachment` TO `app`@`localhost`',
+			'GRANT INSERT, UPDATE ON `application`.`scrape_cache` TO `app`@`localhost`',
+		])
+
+		expect([...granted].sort()).toEqual(['DELETE', 'INSERT', 'SELECT', 'UPDATE'])
+	})
+
+	test('ignores grants on an unrelated database', () => {
+		const granted = parse(['GRANT ALL PRIVILEGES ON `somethingelse`.* TO `app`@`localhost`'])
+		expect(granted.size).toBe(0)
+	})
+
+	test('ignores USAGE, which conveys no privileges', () => {
+		const granted = parse(['GRANT USAGE ON *.* TO `app`@`localhost`'])
+		expect(granted.size).toBe(0)
+	})
+
+	test('handles grants written without backticks', () => {
+		const granted = parse(['GRANT SELECT, DELETE ON application.* TO app@localhost'])
+		expect(granted.has('DELETE')).toBe(true)
+	})
+
+	test('strips column-level qualifiers', () => {
+		const granted = parse(['GRANT SELECT (id, url), UPDATE (text) ON `application`.`attachment` TO `app`@`localhost`'])
+
+		expect(granted.has('SELECT')).toBe(true)
+		expect(granted.has('UPDATE')).toBe(true)
+	})
+
+	test('ignores lines that are not grant statements', () => {
+		expect(parse(['', 'not a grant line', null]).size).toBe(0)
+	})
+})
+
+//
+// The regression that matters most.
+//
+// Schema.js used to issue CREATE TABLE / ALTER TABLE through the application's
+// own connection pool, which failed on production's least-privilege user with
+// "CREATE command denied to user 'app'@'localhost'". Nothing in this module may
+// ever issue DDL again.
+//
+describe('issues no DDL', () => {
+
+	test('verifyScrapeSchema runs only read-only statements', async () => {
+		const statements = []
+
+		// db.promise() hands back a NEW wrapper on every call, so spying on one
+		// instance would miss the one Schema.js obtains internally. Intercept
+		// the factory and instrument each wrapper it returns.
+		const realPromise = db.promise.bind(db)
+		const spy = jest.spyOn(db, 'promise').mockImplementation(() => {
+			const wrapper = realPromise()
+			const originalQuery = wrapper.query.bind(wrapper)
+
+			wrapper.query = (sql, params) => {
+				statements.push(String(sql))
+				return originalQuery(sql, params)
+			}
+
+			return wrapper
+		})
+
+		await Schema.verifyScrapeSchema()
+
+		spy.mockRestore()
+
+		expect(statements.length).toBeGreaterThan(0)
+
+		const forbidden = /\b(CREATE|ALTER|DROP|TRUNCATE|RENAME)\b/i
+		const offenders = statements.filter(statement => forbidden.test(statement))
+
+		expect(offenders).toEqual([])
+	})
+
+	test('the module source contains no DDL outside comments', () => {
+		const fs = require('fs')
+		const path = require('path')
+
+		const source = fs.readFileSync(
+			path.join(__dirname, '..', '..', 'helpers', 'Schema.js'), 'utf8'
+		)
+
+		const code = source
+			.split('\n')
+			.filter(line => !line.trim().startsWith('//'))
+			.join('\n')
+
+		expect(code).not.toMatch(/\bCREATE\s+TABLE\b/i)
+		expect(code).not.toMatch(/\bALTER\s+TABLE\b/i)
+		expect(code).not.toMatch(/\bCREATE\s+INDEX\b/i)
+		expect(code).not.toMatch(/\bDROP\b/i)
+	})
+})