diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cc81df7..af07bb30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,104 @@ ADR-007 and ADR-002. ### Added +- **Audit log retention.** The audit log is append-only and nothing ever removed + a row, so it grew for as long as a site ran - here to the point where the + audit page was tens of thousands of pages deep and therefore useless, because + nobody pages through that to find anything. Site & Facility gains + `audit_retentiondays` plus a card showing the log's size and exactly how many + entries a purge would remove, and there is `flask audit stats` / + `flask audit purge --apply` for the server. **Opt-in: the default is 0, which + keeps everything.** An upgrade that silently started deleting a site's audit + history would be indefensible. Deletion runs in committed batches, because one + `DELETE` over a million rows holds table locks long enough to block the + collectors posting inventory; a run that stops early reports how much is left + rather than claiming it finished. Purging needs its own `audit.purge` + permission - reading the log and destroying it are different acts. + +- **Guides: images, and a reader who cannot see drafts.** Guide images are + fetched through the authenticated API client instead of being pointed at by + ``. An image tag sends no Authorization header and this app keeps its + JWT in one, so every guide image was a broken icon for every user; the same + change makes stored image links survive a subpath mount. A failed image now + says so in words rather than showing a broken icon. Revision and image routes + are gated on their parent guide - revision ids are sequential, so reading one + guide used to be enough to walk the ids and read every unpublished draft on + the site. Search, the guide list and the tag chips now match the PUBLISHED + text, not the working copy, so a reader can no longer get a hit on wording + that is not in the page when they open it. + +### Fixed + +- **A guide's URL no longer moves when its title is edited.** The slug was + regenerated on every save that carried a title, and the editor autosaves every + three seconds - so fixing a typo in a heading silently moved the guide and + broke exactly the pasted links the stored slug exists to protect. The slug is + now set once, at create. + +- **Publishing a guide after a failed save reported success.** `publish()` ran + regardless of whether the pre-publish save reached the server, so the author + saw "Published version N" while readers got the previous revision. A failed + save now stops the publish and says why. + +- **The ADR-015 site-literal check was enforcing nothing.** Its patterns are + assembled from fragments so the published script does not contain the strings + it greps for, but they used the apostrophe-escape form (`'a'\''b'`) instead of + concatenation (`'a''b'`), embedding a literal apostrophe in every pattern. + Nothing could ever match and the check passed everything from the day it was + written. Fixed, and it immediately found 25 real violations - among them a QA + ServiceNow tenant URL sitting in the shipped defaults in seven places. + +- **ServiceNow ships off and blank.** The defaults pointed at one organisation's + QA ServiceNow tenant with the integration enabled, so a site that had never + configured ServiceNow still rendered ticket links - to a tenant its + technicians cannot reach, and for an adopting site, into somebody else's + instance. There is no cross-site default for "which ServiceNow is yours". + +- **Credentials are no longer stored in backup revisions.** The NTLARS/DNC + registry carries an FTP password in cleartext, and it was stored verbatim - + rendered by the revision panel and handed back by the download route as a + ready-to-import `.reg`. Credential values are now redacted at parse, so they + never reach the database, and the rendered file NAMES each withheld value + instead of carrying a placebo one: importing a fake password would produce a + machine that looks restored and cannot connect. `scripts/redact_backup_secrets.py` + scrubs revisions recorded before this (dry run by default). Rotate any + password that was stored. + +- **A search domain that fails is no longer indistinguishable from one that + found nothing.** Each domain catches broadly and returns no rows so one broken + table cannot take the whole search box down - but the technician was then told + they might have misspelled a query that was right, and concluded the record + did not exist. Failed domains are named in the response, shown as a banner, + and suppress the did-you-mean. + +- **An empty `sourcehostname` defeated backup dedup permanently.** The dedup + query maps `''` to `IS NULL` while the insert stored the raw `''`, so those + rows could never match their own chain: every post inserted, the table grew + without bound and the history filled with identical entries. Normalized once + at ingest, whitespace included. + +- **`printedparts` migration 0003 was unguarded.** Unlike its twenty siblings it + ran a bare `ALTER`, so `flask plugin upgrade-all` failed on any database built + by `create_all` and took 0004 down with it. The migration test that should + have caught this only covered single-migration chains; it now covers every + post-cutover chain. + +- **Guide links with several parameters opened the wrong page.** Text is escaped + before formatting, so a captured URL arrives carrying entities and was then + escaped a second time - `&` in every multi-parameter ServiceNow and + SharePoint link. Emphasis could also reach inside an `href`, and + `//host` passed as an in-app link when the browser reads it as off-site. + +- **Retire an asset from the interface.** Machines, PCs, printers and measuring + tools had no way to take a record out of ShopDB without calling the API, so a + scrapped tool stayed in every list and report. The endpoints already existed + and were already soft deletes: the record leaves the lists while its + calibration history, backups and relationships stay put, and it can be + restored by setting `isactive` back. Called Retire rather than Delete for + that reason, and the confirm says what is kept. USB is not included - it + retires through a status column in the external CMMC database and needs its + own decision. + - **`flask catalog dedupe`, `merge` and `rename`: catalog hygiene.** The legacy data carried space-stripped twins - `DellInc.` beside `Dell Inc.`, `Windows10` beside `Windows 10` - which rode into the shipped seed catalog diff --git a/frontend/src/api/index.js b/frontend/src/api/index.js index 0e9e54bb..d6bb4013 100644 --- a/frontend/src/api/index.js +++ b/frontend/src/api/index.js @@ -343,6 +343,11 @@ export const printersApi = { update(id, data) { return api.put(`/printers/${id}`, data) }, + // Soft delete: the server deactivates the asset, so the printer leaves every + // list while its history and relationships stay intact. + remove(id) { + return api.delete(`/printers/${id}`) + }, updateExtension(id, data) { return api.put(`/printers/${id}/printerdata`, data) }, diff --git a/frontend/src/composables/pageTitle.js b/frontend/src/composables/pageTitle.js index b466eb6f..0b5777b0 100644 --- a/frontend/src/composables/pageTitle.js +++ b/frontend/src/composables/pageTitle.js @@ -1,7 +1,7 @@ // The browser tab title, including WHICH record is open. // // The router can only name the KIND of page, because it runs before the view -// has fetched anything: every application read "West Jefferson ShopDB - +// has fetched anything: every application read " ShopDB - // Application", so two tabs on two different machines were indistinguishable, // and a shared link previewed as the section rather than the thing. // @@ -51,7 +51,7 @@ export async function applyGenericTitle(key, page) { if (key !== activeKey) return // navigated on while we awaited // A view that named the record before the facility name arrived composed its // title against the placeholder prefix, so rewrite it rather than leaving - // 'ShopDB - CSF16' on a site called West Jefferson. + // 'ShopDB - CSF16' without the facility name in front of it. document.title = compose(recordTitle !== null ? recordTitle : page) } diff --git a/frontend/src/utils/siteSettings.js b/frontend/src/utils/siteSettings.js index c7ec67ef..a333a950 100644 --- a/frontend/src/utils/siteSettings.js +++ b/frontend/src/utils/siteSettings.js @@ -98,22 +98,20 @@ function darkenHexColor(hex, fraction) { // ServiceNow ticket-link config. Returns the enabled flag, incident/change URL // templates, and the global-search URL template (all with a {ticket} -// placeholder). Defaults mirror the previously-hardcoded GE ServiceNow URLs. +// placeholder). +// +// The defaults are BLANK and the integration ships OFF, matching the server. +// They used to be one organisation's QA ServiceNow tenant, so any site that had +// not configured ServiceNow still rendered ticket links - pointing its +// technicians at a tenant they cannot reach and, for an adopting site, at +// somebody else's. A blank URL renders tickets as plain text, which is the +// correct behaviour for "not configured". export async function getServicenowUrls() { - const enabledRaw = await getSetting('servicenow_enabled', 'true') + const enabledRaw = await getSetting('servicenow_enabled', 'false') const enabled = enabledRaw !== 'false' && enabledRaw !== '0' && enabledRaw !== false - const incidentUrl = await getSetting( - 'servicenow_incident_url', - 'https://geaerospaceqa.service-now.com/now/nav/ui/search/0f8b85d0c7922010099a308dc7c2606a/params/search-term/{ticket}/global-search-data-config-id/c861cea2c7022010099a308dc7c26041/' - ) - const changeUrl = await getSetting( - 'servicenow_change_url', - 'https://geaerospaceqa.service-now.com/now/nav/ui/search/0f8b85d0c7922010099a308dc7c2606a/params/search-term/{ticket}/global-search-data-config-id/c861cea2c7022010099a308dc7c26041/' - ) - const searchUrl = await getSetting( - 'servicenow_search_url', - 'https://geaerospaceqa.service-now.com/now/nav/ui/search/0f8b85d0c7922010099a308dc7c2606a/params/search-term/{ticket}/global-search-data-config-id/c861cea2c7022010099a308dc7c26041/' - ) + const incidentUrl = await getSetting('servicenow_incident_url', '') + const changeUrl = await getSetting('servicenow_change_url', '') + const searchUrl = await getSetting('servicenow_search_url', '') return { enabled, incidentUrl, changeUrl, searchUrl } } diff --git a/frontend/src/views/SearchResults.vue b/frontend/src/views/SearchResults.vue index 1eb53316..3e3d4b41 100644 --- a/frontend/src/views/SearchResults.vue +++ b/frontend/src/views/SearchResults.vue @@ -35,6 +35,18 @@
Searching...
diff --git a/plugins/backups/migrations/versions/0002_backups_lastseenat.py b/plugins/backups/migrations/versions/0002_backups_lastseenat.py index b4f5c70a..e71501a8 100644 --- a/plugins/backups/migrations/versions/0002_backups_lastseenat.py +++ b/plugins/backups/migrations/versions/0002_backups_lastseenat.py @@ -27,8 +27,10 @@ depends_on = None def upgrade(): - columns = {c['name'] for c in - sa.inspect(op.get_bind()).get_columns('backuprevisions')} + inspector = sa.inspect(op.get_bind()) + if 'backuprevisions' not in inspector.get_table_names(): + return + columns = {c['name'] for c in inspector.get_columns('backuprevisions')} if 'lastseenat' not in columns: op.add_column('backuprevisions', sa.Column('lastseenat', sa.DateTime(), nullable=True)) @@ -37,7 +39,9 @@ def upgrade(): def downgrade(): - columns = {c['name'] for c in - sa.inspect(op.get_bind()).get_columns('backuprevisions')} + inspector = sa.inspect(op.get_bind()) + if 'backuprevisions' not in inspector.get_table_names(): + return + columns = {c['name'] for c in inspector.get_columns('backuprevisions')} if 'lastseenat' in columns: op.drop_column('backuprevisions', 'lastseenat') diff --git a/plugins/backups/plugin.py b/plugins/backups/plugin.py index b2e59cc4..cb6f6575 100644 --- a/plugins/backups/plugin.py +++ b/plugins/backups/plugin.py @@ -410,7 +410,13 @@ class BackupsPlugin(BasePlugin): # with_for_update serializes concurrent posts for the same chain: the # fleet collects on one GE-Enforce cycle, so a retry could otherwise # pass the hash check twice and insert the same revision. - sourcehostname = payload.get('sourcehostname') + # Normalize ONCE, and use the same value to query and to store. + # samesource() maps '' to IS NULL, but the insert below used to store + # the raw '' - so a client posting an empty hostname wrote rows that its + # own next post could never match. Every post then deduped against + # nothing, inserting forever: unbounded growth, and a history no + # technician can read. Whitespace does the same thing more quietly. + sourcehostname = (payload.get('sourcehostname') or '').strip() or None latest = (db.session.query(BackupRevision) .filter(BackupRevision.assetid == assetid, BackupRevision.backupkind == kindkey, @@ -476,7 +482,7 @@ class BackupsPlugin(BasePlugin): sharepath=sharepath, sourcefilename=sourcefilename, bytesize=bytesize, - sourcehostname=payload.get('sourcehostname'), + sourcehostname=sourcehostname, collectedat=collectedat or datetime.utcnow(), lastseenat=datetime.utcnow(), ) diff --git a/plugins/backups/services/ntlars.py b/plugins/backups/services/ntlars.py index 6e65ff9d..d0d992c4 100644 --- a/plugins/backups/services/ntlars.py +++ b/plugins/backups/services/ntlars.py @@ -32,6 +32,31 @@ import re SCHEMA = 'ntlars/1' +# SECRETS ARE NOT BACKED UP. +# +# The DNC registry carries an FTP password in cleartext. Stored verbatim it +# reached three places it must not: the backuprevisions table, the revision +# panel (which renders values), and the download route (which hands back a +# ready-to-import .reg). Anyone with backups.view could read the fleet's FTP +# password off a web page. +# +# Redaction happens at PARSE, so the secret never enters the database at all. +# Redacting only on display would leave it at rest and one query away. +# +# What is deliberately NOT done: dropping the value silently. This module's +# parser refuses to discard an unparseable value precisely because a backup that +# looks complete and restores a machine with a setting missing is the worst +# outcome. A redacted value keeps its NAME and records itself in +# `redacted`, so both the panel and a restore can say plainly that this one +# field must be set by hand. +REDACTEDMARKER = '***REDACTED***' +SECRETNAMEPATTERN = re.compile(r'passw|pwd|secret|apikey|token', re.IGNORECASE) + + +def issecretname(name): + """Whether a registry value name holds a credential.""" + return bool(SECRETNAMEPATTERN.search(name or '')) + REGROOT = 'HKEY_LOCAL_MACHINE' DNCPATH = r'SOFTWARE\GE Aircraft Engines\DNC' DNCPATHWOW = r'SOFTWARE\WOW6432Node\GE Aircraft Engines\DNC' @@ -245,6 +270,20 @@ def parse(raw): dialect = 'wow6432node' if sawwow else ('ntlars' if sawplain else 'unknown') + # Strip credentials before the projection is returned - i.e. before anything + # persists it. Recorded by path\name so the omission is visible rather than + # discovered during a restore. + redacted = [] + for path in keys: + for name, value in keys[path].items(): + if not issecretname(name): + continue + if value.get('data') in ('', None): + continue # nothing to hide + keys[path][name] = {'type': value.get('type', 'REG_SZ'), + 'data': REDACTEDMARKER} + redacted.append('{}\\{}'.format(path, name) if path else name) + return { 'schema': SCHEMA, 'sourcedialect': dialect, @@ -252,6 +291,7 @@ def parse(raw): {'path': path, 'values': dict(sorted(keys[path].items()))} for path in sorted(keys) ], + 'redacted': sorted(redacted), } @@ -291,6 +331,12 @@ def render(projection, dialect='ntlars', comments=None): dialect='ntlars' omits WOW6432Node - use with the NTLARS Load... button dialect='wow6432node' includes it - use with `reg import` on 64-bit + + Redacted credentials are rendered as a comment naming the value, NOT as the + marker string. Importing '***REDACTED***' as the FTP password would leave a + machine that looks restored and cannot connect, and the technician would be + debugging the network. A commented-out value fails loudly at the one moment + somebody is looking at the file. """ if dialect not in ('ntlars', 'wow6432node'): raise ValueError('unknown dialect: {!r}'.format(dialect)) @@ -302,10 +348,28 @@ def render(projection, dialect='ntlars', comments=None): if comments: out.append('') + redacted = projection.get('redacted') or [] + if redacted: + out.append('; ---------------------------------------------------------------') + out.append('; {} credential value(s) were NOT backed up and are NOT in this' + .format(len(redacted))) + out.append('; file. Set them by hand after importing:') + for name in redacted: + out.append('; {}'.format(name)) + out.append('; ---------------------------------------------------------------') + out.append('') + for key in projection.get('keys', []): path = key.get('path', '') out.append('[{}]'.format(root + ('\\' + path if path else ''))) for name, entry in (key.get('values') or {}).items(): + # A redacted credential is commented out, never emitted as the + # marker. Importing the marker as a password produces a machine that + # looks restored and cannot log in. + if (entry or {}).get('data') == REDACTEDMARKER: + out.append('; "{}" was not backed up - set it by hand'.format( + _escape(name))) + continue out.append(_rendervalue(name, entry)) out.append('') diff --git a/plugins/computers/frontend/views/PCDetail.vue b/plugins/computers/frontend/views/PCDetail.vue index 09f7a0ba..b288c17c 100644 --- a/plugins/computers/frontend/views/PCDetail.vue +++ b/plugins/computers/frontend/views/PCDetail.vue @@ -5,6 +5,7 @@
Print Label Edit + Back to List
@@ -254,7 +255,7 @@