Skip to content

Fix create_session API endpoint and perform housekeeping on murfey.server.api.session_info - #906

Merged
tieneupin merged 21 commits into
mainfrom
create-session-fix
Oct 12, 2026
Merged

tieneupin merged 21 commits into
mainfrom
create-session-fix

Conversation

@tieneupin

@tieneupin tieneupin commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

NOTE: This PR will need to be deployed alongside DiamondLightSource/murfey-frontend#88

The create_session FastAPI endpoint previously parsed the visit (e.g. 'cm12345-6') and name (an optional long-form description) directly from the URL path. This is unsafe, as the presence of characters such as '/' in the parametrers will break the URL construction logic.

This PR fixes that by passing the visit and name as part of the JSON data payload instead. The API endpoint path was also changed now that visit and name are no longer needed as URL path parts.

As we insert these items using SQLModel, the package will parametrize these strings before insertion, so that is a safe process, However, some sanitisation was nonetheless applied. For the visit, only alphanumerics, hyphens, and underscores will be retained. For the description, the character allow list is expanded to also include:

  • spaces
  • forward slashes
  • round/square brackets
  • periods
  • commas
  • ampersands
  • pluses
  • single/double quotes

As part of housekeeping, the way in which the Murfey database tables were imported and called was also standardised. Instead of importing the tables directly, we use import murfey.util.db as MurfeyDB and called tables from MurfeyDB. Type hints were also replaced with more modern variants.

A bug was also identified when seeding the database fixtures upon startup. For integer primary keys, if a value is manually provided as part of the initial insert, it does not update PostGreSQL's internal counter. Subsequent inserts where the primary key is allow to auto-increment will then attempt to insert into that database with the previous primary key, causing the insert to fail. By removing the primary key from the initial insert in the database fixture, the internal PostgreSQL counter is now updated correctly.

@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.41935% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.43%. Comparing base (5d50503) to head (df3353f).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #906      +/-   ##
==========================================
+ Coverage   55.35%   55.43%   +0.07%     
==========================================
  Files         103      103              
  Lines       11632    11636       +4     
  Branches     1546     1546              
==========================================
+ Hits         6439     6450      +11     
+ Misses       4844     4837       -7     
  Partials      349      349              
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tieneupin tieneupin changed the title Create session fix Fix create_session API endpoint and perform housekeeping on murfey.server.api.session_info Oct 1, 2026
@tieneupin
tieneupin marked this pull request as ready for review October 1, 2026 14:38
@tieneupin tieneupin self-assigned this Oct 1, 2026
@tieneupin tieneupin added bug Something isn't working server Relates to the server component labels Oct 1, 2026
Comment thread src/murfey/server/api/session_info.py Outdated
name=name,
visit=visit,
session = MurfeyDB.Session(
name=session_info.name,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name definitely needs sanitising - I'd suggest stripping out anything except standard alphanumeric and _ or -

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I've tacked on the sanitise() function to both of them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've applied sanitisation, albeit slightly milder. Besides alphanumerics, hyphens, and underscores, the character allow list is expanded to also include:

  • spaces
  • forward slashes
  • round/square brackets
  • periods
  • commas
  • ampersands
  • pluses
  • single/double quotes

Because we're not writing an SQL statement directly, but rather inserting it using the sqlalchemy and sqlmodel functions, the insert will be safe.

Comment thread src/murfey/server/api/session_info.py Outdated
visit=visit,
session = MurfeyDB.Session(
name=session_info.name,
visit=session_info.visit,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably also worth sanitising the visit in the same way

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've decided to sanitise the visit thoroughly, so that only alphanumerics, underscores, and hyphens are retained.

@tieneupin
tieneupin requested a review from d-j-hatton October 5, 2026 09:21
@tieneupin
tieneupin merged commit 739dd4d into main Oct 12, 2026
9 checks passed
@tieneupin
tieneupin deleted the create-session-fix branch October 12, 2026 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working server Relates to the server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants