Deploy and schema updates for testing

deploy.sh

@@ -48,6 +48,37 @@
 #
 # Override the location for a staging box with:  WEBROOT=/var/www/ss-staging ./deploy.sh
 #
+# --- the database account ------------------------------------------------
+#
+# The API connects as a least-privilege account with DML rights only. This is
+# deliberate: the server fetches untrusted URLs for link previews, and a
+# service able to reshape or drop its own tables is a far larger blast radius
+# than that feature warrants.
+#
+#   CREATE USER 'app'@'localhost' IDENTIFIED BY '<password>';
+#   GRANT SELECT, INSERT, UPDATE, DELETE ON application.* TO 'app'@'localhost';
+#   FLUSH PRIVILEGES;
+#
+#   -- deliberately NOT granted: CREATE, ALTER, DROP, INDEX
+#
+# Then set DB_USER=app and DB_PASS=<password> in ~/.env. Note the asymmetry:
+# the admin scripts here (backupDatabase.sh, applyLastBackup.sh,
+# applyScrapeSchema.sh) still connect as root with $PROD_DB_PASS.
+#
+# DELETE is genuinely required -- the scraper drops shell attachment rows for
+# dead links and purges expired cache entries.
+#
+# --- schema changes ------------------------------------------------------
+#
+# Because the app holds no DDL, a deploy that introduces schema changes needs
+# one manual step, run once per environment as a database admin:
+#
+#   ./applyScrapeSchema.sh
+#
+# It is idempotent, so re-running is safe. The API verifies the schema and the
+# grants at boot; if either is missing it logs the remedy and serves normally
+# with link previews disabled.
+#
 
 set -euo pipefail
 

server/helpers/Schema.js

@@ -1,58 +1,57 @@
 //
-// Idempotent schema convergence for the scraper tables.
+// Verification of the scraper's schema and privileges. Issues NO DDL.
 //
-// There is no migration framework in this project and no schema file in git,
+// This module used to converge the schema at boot with CREATE TABLE / ALTER
-// so this runs at boot, checks information_schema for each object, and applies
+// TABLE. That worked only because dev connects as root (shell.nix exports
-// only what is missing. Dev and prod self-heal, and deploy.sh needs no change.
+// DB_USER=root); on production, which runs as a least-privilege `app` user, it
+// failed with:
 //
-// Portability matters here: dev is MariaDB 11.4 and prod is MySQL 8.
+//   CREATE command denied to user 'app'@'localhost' for table scrape_cache
-// ADD COLUMN IF NOT EXISTS is MariaDB-only and is deliberately not used -
+//
-// every ALTER is guarded by an explicit existence check instead.
+// The restricted account is correct - the server ingests untrusted URLs and
+// should not be able to reshape its own tables. So DDL moved out of the
+// application entirely: an admin runs ./applyScrapeSchema.sh once, and this
+// module only checks that the result is present and usable.
+//
+// Note on why the DDL is REMOVED rather than guarded by an existence check:
+// MariaDB evaluates the CREATE privilege before IF NOT EXISTS, so
+// `CREATE TABLE IF NOT EXISTS` would keep erroring for a DML-only user on every
+// boot even once the table exists.
 //
 
 const db = require('@config/database')
 
 let Schema = module.exports = {}
 
-const SCRAPE_CACHE_TABLE = `
+const REQUIRED_TABLE = 'scrape_cache'
-	CREATE TABLE IF NOT EXISTS scrape_cache (
+const REQUIRED_COLUMNS = ['scrape_state', 'scrape_attempts', 'enqueued_at']
-		id            int(10) unsigned NOT NULL AUTO_INCREMENT,
+const REQUIRED_INDEX = 'attachment_scrape_state'
-		url_hash      char(64) NOT NULL,
-		url           varchar(1000) NOT NULL DEFAULT '',
-		canonical_url varchar(1000) DEFAULT NULL,
-		status        smallint(6) NOT NULL DEFAULT 0,
-		title         varchar(512) DEFAULT NULL,
-		site_name     varchar(255) DEFAULT NULL,
-		description   mediumtext DEFAULT NULL,
-		keywords      varchar(512) DEFAULT NULL,
-		search_text   mediumtext DEFAULT NULL,
-		image_url     varchar(1000) DEFAULT NULL,
-		file_location varchar(500) DEFAULT NULL,
-		etag          varchar(255) DEFAULT NULL,
-		last_modified varchar(128) DEFAULT NULL,
-		content_type  varchar(128) DEFAULT NULL,
-		fetch_state   varchar(20) NOT NULL DEFAULT 'pending',
-		failure_count int(11) NOT NULL DEFAULT 0,
-		last_error    varchar(255) DEFAULT NULL,
-		fetched_at    int(11) DEFAULT NULL,
-		expires_at    int(11) DEFAULT NULL,
-		PRIMARY KEY (id),
-		UNIQUE KEY scrape_cache_url_hash (url_hash),
-		KEY scrape_cache_expires_at (expires_at),
-		KEY scrape_cache_fetch_state (fetch_state)
-	) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4
-`
 
-// DEFAULT 2 ("done") keeps existing rows out of the boot recovery sweep.
+// The scraper writes and deletes, not just reads. DELETE is genuinely needed:
-const ATTACHMENT_COLUMNS = [
+// terminal failures drop the shell attachment row (ScrapeQueue), and
-	{ name: 'scrape_state', ddl: 'ALTER TABLE attachment ADD COLUMN scrape_state tinyint(4) NOT NULL DEFAULT 2' },
+// ScrapeCache.purgeExpired removes expired rows.
-	{ name: 'scrape_attempts', ddl: 'ALTER TABLE attachment ADD COLUMN scrape_attempts tinyint(4) NOT NULL DEFAULT 0' },
+const REQUIRED_PRIVILEGES = ['SELECT', 'INSERT', 'UPDATE', 'DELETE']
-	{ name: 'enqueued_at', ddl: 'ALTER TABLE attachment ADD COLUMN enqueued_at int(11) DEFAULT NULL' },
-]
 
-const ATTACHMENT_INDEXES = [
+// Set by verifyScrapeSchema so the per-request paths can check a flag rather
-	{ name: 'attachment_scrape_state', ddl: 'ALTER TABLE attachment ADD KEY attachment_scrape_state (scrape_state, enqueued_at)' },
+// than querying information_schema on every note save.
-]
+let ready = false
+let lastResult = null
+
+Schema.isScrapeSchemaReady = () => ready
+Schema.lastVerification = () => lastResult
+
+// Test seam.
+Schema.__setReady = (value) => { ready = !!value }
+Schema.__reset = () => { ready = false; lastResult = null }
+
+const tableExists = async (table) => {
+	const [rows] = await db.promise().query(
+		`SELECT 1 FROM information_schema.TABLES
+		 WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = ? LIMIT 1`,
+		[table]
+	)
+	return rows.length > 0
+}
 
 const columnExists = async (table, column) => {
 	const [rows] = await db.promise().query(
@@ -71,34 +70,141 @@ const indexExists = async (table, index) => {
 	)
 	return rows.length > 0
 }
+
+Schema.__tableExists = tableExists
 Schema.__columnExists = columnExists
 Schema.__indexExists = indexExists
 
 //
-// Safe to call repeatedly. Returns the list of statements it actually applied,
+// Parse SHOW GRANTS output into the set of privileges that apply to this
-// so boot logging says something useful on the one run that matters.
+// database. Any user may run SHOW GRANTS FOR CURRENT_USER(), so this needs no
+// privileges of its own.
 //
-Schema.ensureScrapeSchema = async () => {
+// Grant lines look like:
-	const applied = []
+//   GRANT ALL PRIVILEGES ON *.* TO `root`@`localhost` WITH GRANT OPTION
+//   GRANT SELECT, INSERT, UPDATE, DELETE ON `application`.* TO `app`@`localhost`
+//   GRANT SELECT ON `application`.`attachment` TO `app`@`localhost`
+//   GRANT USAGE ON *.* TO `app`@`localhost`
+//
+const PRIVILEGE_PATTERN = /^GRANT\s+(.+?)\s+ON\s+(\S+)\s+TO\s/i
 
-	await db.promise().query(SCRAPE_CACHE_TABLE)
+Schema.parseGrants = (lines, database, tables) => {
+	const granted = new Set()
 
-	for (const column of ATTACHMENT_COLUMNS) {
+	// Scope strings that cover our tables. Backticks are optional in output.
-		if (await columnExists('attachment', column.name)) { continue }
+	const strip = (value) => value.replace(/`/g, '')
-		await db.promise().query(column.ddl)
+	const relevantScopes = new Set([
-		applied.push(`attachment.${column.name}`)
+		'*.*',
+		`${database}.*`,
+		...tables.map(table => `${database}.${table}`),
+	])
+
+	for (const line of lines) {
+		const match = String(line).match(PRIVILEGE_PATTERN)
+		if (!match) { continue }
+
+		const scope = strip(match[2])
+		if (!relevantScopes.has(scope)) { continue }
+
+		// Drop any column-level qualifiers, e.g. "SELECT (id, url)".
+		const privileges = match[1].replace(/\s*\([^)]*\)/g, '')
+
+		for (const privilege of privileges.split(',')) {
+			const name = privilege.trim().toUpperCase()
+			if (!name || name === 'USAGE') { continue }
+
+			if (name === 'ALL PRIVILEGES' || name === 'ALL') {
+				REQUIRED_PRIVILEGES.forEach(required => granted.add(required))
+				continue
+			}
+
+			granted.add(name)
+		}
 	}
 
-	// Index last: it references the columns above.
+	return granted
-	for (const index of ATTACHMENT_INDEXES) {
+}
-		if (await indexExists('attachment', index.name)) { continue }
+
-		await db.promise().query(index.ddl)
+const checkPrivileges = async () => {
-		applied.push(`attachment KEY ${index.name}`)
+	let lines = []
+
+	try {
+		const [rows] = await db.promise().query('SHOW GRANTS FOR CURRENT_USER()')
+		// Each row is an object with a single, server-named column.
+		lines = rows.map(row => Object.values(row)[0])
+	} catch (error) {
+		// If we cannot read our own grants, do not block on a guess - the
+		// object checks above are the real gate.
+		console.log('[schema] could not read grants, skipping privilege check:', error.message)
+		return { missing: [], database: null, currentUser: null }
+	}
+
+	const [dbRows] = await db.promise().query('SELECT DATABASE() AS name, CURRENT_USER() AS who')
+	const database = dbRows[0] && dbRows[0].name
+	const currentUser = dbRows[0] && dbRows[0].who
+
+	const granted = Schema.parseGrants(lines, database, ['attachment', REQUIRED_TABLE])
+
+	return {
+		missing: REQUIRED_PRIVILEGES.filter(privilege => !granted.has(privilege)),
+		database,
+		currentUser,
+	}
+}
+
+// 'app@localhost' -> "'app'@'localhost'", so the remedy is copy-pasteable.
+const quoteUser = (currentUser) => {
+	if (!currentUser) { return '<app user>' }
+
+	const [name, host] = String(currentUser).split('@')
+	return `'${name}'@'${host || 'localhost'}'`
+}
+
+//
+// Check that everything the scraper needs is present. Never mutates anything.
+//
+// Returns { ok, missingObjects, missingPrivileges, remedy }.
+//
+Schema.verifyScrapeSchema = async () => {
+	const missingObjects = []
+
+	if (!await tableExists(REQUIRED_TABLE)) {
+		missingObjects.push(`table ${REQUIRED_TABLE}`)
+	}
+
+	for (const column of REQUIRED_COLUMNS) {
+		if (!await columnExists('attachment', column)) {
+			missingObjects.push(`attachment.${column}`)
+		}
+	}
+
+	if (!await indexExists('attachment', REQUIRED_INDEX)) {
+		missingObjects.push(`attachment index ${REQUIRED_INDEX}`)
+	}
+
+	const privileges = await checkPrivileges()
+	const missingPrivileges = privileges.missing || []
+
+	const remedy = []
+	if (missingObjects.length) {
+		remedy.push('run ./applyScrapeSchema.sh as a database admin')
+	}
+	if (missingPrivileges.length) {
+		// Name the real database and user so this can be pasted straight into
+		// a mysql prompt rather than decoded first.
+		const target = `${privileges.database || '<database>'}.*`
+		remedy.push(
+			`GRANT ${missingPrivileges.join(', ')} ON ${target} TO ${quoteUser(privileges.currentUser)}`
+		)
 	}
 
-	if (applied.length) {
+	lastResult = {
-		console.log('[schema] applied scrape schema changes:', applied.join(', '))
+		ok: missingObjects.length === 0 && missingPrivileges.length === 0,
+		missingObjects,
+		missingPrivileges,
+		remedy: remedy.join('; '),
 	}
+	ready = lastResult.ok
 
-	return applied
+	return lastResult
 }

server/index.js

@@ -348,16 +348,34 @@ app.use('/api/metric-tracking', metricTracking)
 //
 // Scraper startup.
 //
-// ensureScrapeSchema() converges the scrape_cache table and the attachment
+// The application deliberately holds no DDL privileges, so it does not create
-// lifecycle columns. It is idempotent and runs before the listener binds, so
+// its own schema - verifyScrapeSchema() only checks that an admin has already
-// there is no window where requests arrive against a schema that is mid-change.
+// run ./applyScrapeSchema.sh and that the required DML grants are present.
-// There is no migration framework in this project - see server/helpers/Schema.js.
+//
+// When anything is missing the scraper stays off rather than failing an INSERT
+// on every note save. The listener binds either way: the API must not depend on
+// link previews being available.
 //
 const Schema = require('@helpers/Schema')
 const ScrapeQueue = require('@workers/ScrapeQueue')
 
-Schema.ensureScrapeSchema()
+Schema.verifyScrapeSchema()
-	.then(() => {
+	.then(result => {
+		if (!result.ok) {
+			const missing = [
+				...result.missingObjects,
+				...result.missingPrivileges.map(privilege => `privilege ${privilege}`),
+			].join(', ')
+
+			console.log(`Scraper: DISABLED - missing ${missing}. To fix: ${result.remedy}`)
+			return null
+		}
+
+		// Say so positively. Previously a healthy scraper logged nothing, so
+		// "started" and "silently disabled" looked identical in pm2 logs --
+		// which is how a dead scraper went unnoticed on production.
+		console.log('Scraper: ready')
+
 		ScrapeQueue.start()
 
 		// The queue lives in memory, so a pm2 reload loses whatever was in it.
@@ -372,7 +390,7 @@ Schema.ensureScrapeSchema()
 	})
 	.catch(error => {
 		// A scraper that cannot start must not stop the API from serving.
-		console.log('Scraper: startup failed, link previews disabled this boot:', error.message)
+		console.log('Scraper: startup check failed, link previews disabled this boot:', error.message)
 	})
 	.finally(() => {
 		//Output running status

server/migrations/0001_scrape_cache_and_state.sql

@@ -1,18 +1,32 @@
 --
 -- Scrape cache + per-attachment scrape lifecycle.
 --
--- This file is the human-readable record. It is NOT applied by hand: the same
+-- HOW THIS IS APPLIED
--- statements live in server/helpers/Schema.js and are applied idempotently at
--- boot after an information_schema existence check, so dev and prod converge
--- without a migration runner.
 --
--- Portability: dev is MariaDB 11.4, prod is MySQL 8. Nothing here may use
+--   ./applyScrapeSchema.sh      (from the repo root, as an admin)
--- ADD COLUMN IF NOT EXISTS (MariaDB-only).
+--
+-- Run it once per environment. The application does NOT apply this itself:
+-- production connects as a least-privilege `app` user with DML rights only, and
+-- deliberately holds no CREATE/ALTER. server/helpers/Schema.js only *verifies*
+-- that these objects exist and that the required privileges are held; if they
+-- are missing it disables link previews and logs the remedy.
+--
+-- Safe to re-run: every statement is conditional.
+--
+-- Portability: dev and production are both MariaDB 11.4, so ADD COLUMN IF NOT
+-- EXISTS and CREATE INDEX IF NOT EXISTS are available. If this project ever
+-- moves to MySQL 8 these become invalid and need information_schema guards
+-- instead.
 --
 
--- One row per URL, shared across users and notes. url_hash carries the UNIQUE
+-- One row per URL, shared across users and notes.
--- index because a UNIQUE on varchar(1000) utf8mb4 is 4000 bytes and exceeds
+--
--- InnoDB's 3072-byte key limit.
+-- url_hash carries the UNIQUE index because a UNIQUE on varchar(1000) utf8mb4
+-- is 4000 bytes and exceeds InnoDB's 3072-byte key limit. `url` is kept
+-- human-readable for debugging only.
+--
+-- COLLATE is stated explicitly to match the existing tables (utf8mb4_general_ci)
+-- rather than inheriting whatever the server default happens to be.
 CREATE TABLE IF NOT EXISTS `scrape_cache` (
   `id`            int(10) unsigned NOT NULL AUTO_INCREMENT,
   `url_hash`      char(64) NOT NULL,
@@ -38,14 +52,16 @@ CREATE TABLE IF NOT EXISTS `scrape_cache` (
   UNIQUE KEY `scrape_cache_url_hash` (`url_hash`),
   KEY `scrape_cache_expires_at` (`expires_at`),
   KEY `scrape_cache_fetch_state` (`fetch_state`)
-) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;
 
--- Per-attachment lifecycle, so an in-memory queue survives a restart.
+-- Per-attachment lifecycle, so the in-memory scrape queue survives a restart.
 --
 -- scrape_state: 0 = pending, 1 = fetching, 2 = done, 3 = failed.
--- DEFAULT 2 is load-bearing: it keeps the ~2200 pre-existing rows out of the
+--
--- recovery sweep on first boot.
+-- DEFAULT 2 is load-bearing: it keeps every pre-existing attachment row
-ALTER TABLE `attachment` ADD COLUMN `scrape_state`    tinyint(4) NOT NULL DEFAULT 2;
+-- (~2200 in production) out of the boot recovery sweep.
-ALTER TABLE `attachment` ADD COLUMN `scrape_attempts` tinyint(4) NOT NULL DEFAULT 0;
+ALTER TABLE `attachment` ADD COLUMN IF NOT EXISTS `scrape_state`    tinyint(4) NOT NULL DEFAULT 2;
-ALTER TABLE `attachment` ADD COLUMN `enqueued_at`     int(11) DEFAULT NULL;
+ALTER TABLE `attachment` ADD COLUMN IF NOT EXISTS `scrape_attempts` tinyint(4) NOT NULL DEFAULT 0;
-ALTER TABLE `attachment` ADD KEY `attachment_scrape_state` (`scrape_state`, `enqueued_at`);
+ALTER TABLE `attachment` ADD COLUMN IF NOT EXISTS `enqueued_at`     int(11) DEFAULT NULL;
+
+CREATE INDEX IF NOT EXISTS `attachment_scrape_state` ON `attachment` (`scrape_state`, `enqueued_at`);

server/models/Attachment.js

@@ -288,11 +288,18 @@ Attachment.generateThumbnail = (fileName) => {
 Attachment.scanTextForWebsites = (userId, noteId, noteText) => {
 
 	const ScrapeQueue = require('@workers/ScrapeQueue')
+	const Schema = require('@helpers/Schema')
 
 	// Fixed: the old version called resolve() without returning, so an empty
 	// note fell through and ran the DB lookup anyway.
 	if (!noteText || noteText.length == 0) { return Promise.resolve('') }
 
+	// When the scrape schema is absent the new columns do not exist, so
+	// creating a pending row would fail on every save. The diff and soft-delete
+	// below still run - they touch no new columns - so removing a link from a
+	// note behaves exactly as before.
+	const scrapeAvailable = Schema.isScrapeSchemaReady()
+
 	let knownText = ''
 
 	return Attachment.urlForNote(userId, noteId)
@@ -320,6 +327,10 @@ Attachment.scanTextForWebsites = (userId, noteId, noteText) => {
 
 			if (!remaining.length) { return knownText }
 
+			// No schema, no previews. Return what is already cached rather than
+			// attempting inserts that cannot succeed.
+			if (!scrapeAvailable) { return knownText }
+
 			// Create a shell row per new URL, then hand off to the worker.
 			return Promise.all(remaining.map(url =>
 				Attachment.createPendingUrlAttachment(userId, noteId, url)
@@ -378,8 +389,10 @@ Attachment.createPendingUrlAttachment = (userId, noteId, url) => {
 Attachment.scrapeUrlsCreateAttachments = (userId, noteId, foundUrls) => {
 
 	const ScrapeQueue = require('@workers/ScrapeQueue')
+	const Schema = require('@helpers/Schema')
 
 	if (!foundUrls || !foundUrls.length) { return Promise.resolve('') }
+	if (!Schema.isScrapeSchemaReady()) { return Promise.resolve('') }
 
 	return Promise.all(foundUrls.map(url =>
 		Attachment.createPendingUrlAttachment(userId, noteId, url)
@@ -562,6 +575,11 @@ Attachment.getPushkeyBookmarklet = (userId) => {
 Attachment.pushUrl = (pushkey, url) => {
 
 	const ScrapeQueue = require('@workers/ScrapeQueue')
+	const Schema = require('@helpers/Schema')
+
+	// Preserve the bookmarklet contract - resolve truthy, leaking nothing -
+	// rather than failing an insert against columns that do not exist.
+	if (!Schema.isScrapeSchemaReady()) { return Promise.resolve(true) }
 
 	let userId = null
 	pushkey = String(pushkey || '').replace(/ /g, '+')

server/tests/models/Attachment.test.js

@@ -1,6 +1,7 @@
 const Attachment = require('../../models/Attachment')
 const User = require('../../models/User')
 const ScrapeQueue = require('@workers/ScrapeQueue')
+const Schema = require('@helpers/Schema')
 const Robots = require('@helpers/Robots')
 const FetchMock = require('../support/fetchMock')
 const dns = require('dns').promises
@@ -82,6 +83,8 @@ describe('Push URL', () => {
 
 	beforeEach(() => {
 		FetchMock.install()
+		// The server marks this at boot; there is no boot phase in tests.
+		Schema.__setReady(true)
 		Robots.__resetCache()
 		dnsSpy = jest.spyOn(dns, 'lookup').mockResolvedValue([{ address: '93.184.216.34', family: 4 }])
 		// Real code path, no politeness waiting and no robots lookups.
@@ -92,6 +95,7 @@ describe('Push URL', () => {
 		FetchMock.restore()
 		dnsSpy.mockRestore()
 		ScrapeQueue.__reset()
+		Schema.__reset()
 	})
 
 	test('scrapes a pushed url into a searchable attachment', () => {

server/tests/models/ScrapeQueue.test.js

@@ -17,6 +17,7 @@ const testUserName = 'jestTestUserScrapeQueue'
 const password = 'Beans19934!!!'
 
 let userId = null
+let noteId = null
 let dnsSpy = null
 
 const page = (title, description) => FetchMock.html(`
@@ -35,7 +36,11 @@ const attachmentRow = (id) => db.promise()
 	.then(([rows]) => rows[0] || null)
 
 beforeAll(async () => {
-	await Schema.ensureScrapeSchema()
+	// The server normally does this at boot; tests have no boot phase.
+	const verification = await Schema.verifyScrapeSchema()
+	if (!verification.ok) {
+		throw new Error(`dev scrape schema not applied - run ./applyScrapeSchema.sh (${verification.remedy})`)
+	}
 
 	try {
 		const existing = await User.getByUserName(testUserName)
@@ -44,10 +49,19 @@ beforeAll(async () => {
 
 	const registered = await User.register(testUserName, password)
 	userId = registered.userId
+
+	// A real note, so the soft-delete path below sees an existing note and
+	// hides the attachment rather than deleting it.
+	const [note] = await db.promise().query(
+		'INSERT INTO note (user_id, created, opened) VALUES (?, ?, ?)',
+		[userId, Math.round(Date.now() / 1000), Math.round(Date.now() / 1000)]
+	)
+	noteId = note.insertId
 })
 
 beforeEach(() => {
 	FetchMock.install()
+	Schema.__setReady(true)
 	Robots.__resetCache()
 	dnsSpy = jest.spyOn(dns, 'lookup').mockResolvedValue([{ address: '93.184.216.34', family: 4 }])
 	HttpFetch.__setSleep(() => Promise.resolve())
@@ -65,6 +79,7 @@ afterEach(() => {
 
 afterAll(async () => {
 	await db.promise().query('DELETE FROM attachment WHERE user_id = ?', [userId])
+	await db.promise().query('DELETE FROM note WHERE user_id = ?', [userId])
 	db.end()
 })
 
@@ -305,6 +320,92 @@ describe('robots.txt', () => {
 	})
 })
 
+//
+// Production ran for a while with the scrape columns missing, and every note
+// save attempted an INSERT that could not succeed. These pin the degraded
+// behaviour: do nothing, quietly, and leave everything else working.
+//
+describe('degrades cleanly when the scrape schema is absent', () => {
+
+	beforeEach(() => {
+		Schema.__setReady(false)
+	})
+
+	test('scanTextForWebsites creates no rows and makes no requests', async () => {
+		FetchMock.route(/.*/, () => page('Should Not Be Fetched'))
+
+		const [before] = await db.promise().query(
+			'SELECT COUNT(*) AS n FROM attachment WHERE user_id = ?', [userId])
+
+		const text = await Attachment.scanTextForWebsites(userId, null, 'see https://example.com/unavailable')
+
+		const [after] = await db.promise().query(
+			'SELECT COUNT(*) AS n FROM attachment WHERE user_id = ?', [userId])
+
+		expect(after[0].n).toBe(before[0].n)
+		expect(FetchMock.callCount()).toBe(0)
+		expect(typeof text).toBe('string')
+	})
+
+	//
+	// Removing a link from a note must still clean up its attachment. That path
+	// touches none of the new columns, so it is unaffected by the outage.
+	//
+	// Note the existing semantics: scanTextForWebsites calls
+	// Attachment.delete(..., urlDelete = true), which HARD deletes. The
+	// visible = 0 soft-delete applies only when a user removes an attachment
+	// directly while its note still exists (urlDelete = false).
+	//
+	test('still removes an attachment whose url left the note', async () => {
+		Schema.__setReady(true)
+		const url = 'https://example.com/will-be-removed'
+		await clearCache(url)
+		FetchMock.route(url, () => page('Removed Later'))
+
+		const attachmentId = await Attachment.createPendingUrlAttachment(userId, noteId, url)
+		await ScrapeQueue.enqueueUrl({ userId, noteId, attachmentId, url })
+		expect(await attachmentRow(attachmentId)).not.toBeNull()
+
+		// Now the schema "disappears" and the note is saved without that link.
+		Schema.__setReady(false)
+		await Attachment.scanTextForWebsites(userId, noteId, 'no links here any more')
+
+		// Attachment.delete is deliberately fire-and-forget in scanTextForWebsites,
+		// so give it a tick to land.
+		await new Promise(resolve => setTimeout(resolve, 150))
+
+		expect(await attachmentRow(attachmentId)).toBeNull()
+	})
+
+	test('scrapeUrlsCreateAttachments resolves empty without touching the DB', async () => {
+		FetchMock.route(/.*/, () => page('nope'))
+
+		await expect(Attachment.scrapeUrlsCreateAttachments(userId, null, ['https://example.com/x']))
+			.resolves.toBe('')
+		expect(FetchMock.callCount()).toBe(0)
+	})
+
+	test('pushUrl keeps its contract and resolves truthy', async () => {
+		FetchMock.route(/.*/, () => page('nope'))
+
+		await expect(Attachment.pushUrl('any-key', 'https://example.com/x')).resolves.toBe(true)
+		expect(FetchMock.callCount()).toBe(0)
+	})
+
+	test('recoverPending returns zero without querying', async () => {
+		await expect(ScrapeQueue.recoverPending()).resolves.toEqual({ requeued: 0, retired: 0 })
+	})
+
+	test('enqueueUrl is a no-op backstop', async () => {
+		FetchMock.route(/.*/, () => page('nope'))
+
+		await expect(ScrapeQueue.enqueueUrl({
+			userId, noteId: null, attachmentId: null, url: 'https://example.com/x',
+		})).resolves.toBe('')
+		expect(FetchMock.callCount()).toBe(0)
+	})
+})
+
 describe('crash recovery', () => {
 
 	test('re-enqueues rows left pending by a restart', async () => {

server/workers/ScrapeQueue.js

@@ -21,6 +21,7 @@ const HttpFetch = require('@helpers/HttpFetch')
 const HostQueue = require('@helpers/HostQueue')
 const Robots = require('@helpers/Robots')
 const Realtime = require('@helpers/Realtime')
+const Schema = require('@helpers/Schema')
 const SiteScrape = require('@helpers/SiteScrape')
 const ScrapeCache = require('@models/ScrapeCache')
 
@@ -354,6 +355,10 @@ ScrapeQueue.enqueueUrl = ({ userId, noteId, attachmentId, url }) => {
 const runJob = async ({ userId, noteId, attachmentId, url }) => {
 	if (!settings.enabled) { return '' }
 
+	// Backstop, so no future caller can reach the DB layer unguarded when the
+	// scrape columns are absent.
+	if (!Schema.isScrapeSchemaReady()) { return '' }
+
 	const pending = perUser.get(userId) || 0
 	if (pending >= settings.maxPendingPerUser) {
 		// Left at scrape_state = 0, so recoverPending() picks it up later.
@@ -426,6 +431,10 @@ ScrapeQueue.awaitUrl = async (promise, capMs) => {
 ScrapeQueue.recoverPending = async () => {
 	if (!settings.enabled) { return { requeued: 0, retired: 0 } }
 
+	// Every query below references scrape_state, which only exists once an
+	// admin has run ./applyScrapeSchema.sh.
+	if (!Schema.isScrapeSchemaReady()) { return { requeued: 0, retired: 0 } }
+
 	// Anything left at FETCHING died with the previous process.
 	await db.promise().query(
 		'UPDATE attachment SET scrape_state = ? WHERE scrape_state = ?',