Skip to content

Save BioModel Endpoint#1500

Merged
Ezequiel-Valencia merged 18 commits into
masterfrom
save-biomodel-endpoint
May 15, 2025
Merged

Save BioModel Endpoint#1500
Ezequiel-Valencia merged 18 commits into
masterfrom
save-biomodel-endpoint

Conversation

@Ezequiel-Valencia

@Ezequiel-Valencia Ezequiel-Valencia commented May 8, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

This branch does the following:

  • Endpoints alongside tests for both saving and deleting BioModel's
  • Make the Java GUI client now use the rest endpoint for saving and deleting a BioModel
  • Set DataAccessException to the HTTP error code 500, such that the GUI client can now recreate the same errors required
  • Improve client error handling, such that now a message is displayed if it's the clients fault for communication, rather than just the server.
  • Improve server errors for the User endpoint
  • Include a Custom Object Mapper for the client such that XML is now read directly as a string rather than JSON.

@Ezequiel-Valencia
Update User DB Error Handling
aacfc51
@Ezequiel-Valencia
Save/Delete BioModel Endpoint And Tests
5f220ab
@Ezequiel-Valencia
Generated Clients
b59f217
@Ezequiel-Valencia
Add Data Access Exception HTTP Error Code
515f2f9
@Ezequiel-Valencia
Update Name of Get BioModel
2419fc9
@Ezequiel-Valencia
Java GUI Client Uses New Save and Delete BioModel Endpoints
67af4b9
@Ezequiel-Valencia
Update BioModel Endpoint to Have Only One Save
de6c399 
Instead of several save variances, allow for only one save endpoint with optional parameters.
@Ezequiel-Valencia
Custom Object Mapper
b444c21 
Use a custom object mapper for the Java client so that XML strings can be properly read, and further enhancements can be made on mapping relations.
@Ezequiel-Valencia
Update Save Tests For Validation
e1dd610 
Update the save tests to ensure input validation is occurring, and follow the new single save endpoint.
@Ezequiel-Valencia
Updated Client
74be067 
Update generated client.
@Ezequiel-Valencia
Fix Endpoint Test For BioModel Save
6a003c2
@Ezequiel-Valencia
Force Specific Name for JSON Property
2024ad4
@Ezequiel-Valencia
Updated Clients
f3d3f6f
@Ezequiel-Valencia
Change Data Access Exception HTTP Code to 500
93623e6
@Ezequiel-Valencia Ezequiel-Valencia self-assigned this May 8, 2025
@Ezequiel-Valencia Ezequiel-Valencia linked an issue May 8, 2025 that may be closed by this pull request
Implement BioModel Endpoints #1491
Closed
3 tasks
@jcschaff

jcschaff commented May 13, 2025

Copy link
Copy Markdown
Member

please look at CodeQL failure - we need to decide to fix or ignore.

jcschaff
jcschaff approved these changes May 13, 2025
View reviewed changes

@jcschaff jcschaff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nice job, reviewed with Ezequiel

github-advanced-security[bot]
github-advanced-security AI found potential problems May 15, 2025
View reviewed changes
Comment thread vcell-core/src/main/java/cbit/util/xml/XmlUtil.java
}
sDoc = builder.build(reader);
builder.setFeature("https://apache.org/xml/features/disallow-doctype-decl", true); // disable potential attack vector https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#java
sDoc = builder.build(reader);

Check failure

Code scanning / CodeQL

Resolving XML external entity in user-controlled data

XML parsing depends on a [user-provided value](1) without guarding against external entity expansion. XML parsing depends on a [user-provided value](2) without guarding against external entity expansion. XML parsing depends on a [user-provided value](3) without guarding against external entity expansion.

Copilot Autofix

AI about 1 year ago

To fix the issue, we need to explicitly disable DTDs and external entities in the SAXBuilder instance used in the readXML method. This can be achieved by setting the http://apache.org/xml/features/disallow-doctype-decl feature to true and the http://xml.org/sax/features/external-general-entities and http://xml.org/sax/features/external-parameter-entities features to false. These changes ensure that the XML parser does not process external entities, effectively mitigating the risk of XXE attacks.

Suggested changeset 1
vcell-core/src/main/java/cbit/util/xml/XmlUtil.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/vcell-core/src/main/java/cbit/util/xml/XmlUtil.java b/vcell-core/src/main/java/cbit/util/xml/XmlUtil.java
--- a/vcell-core/src/main/java/cbit/util/xml/XmlUtil.java
+++ b/vcell-core/src/main/java/cbit/util/xml/XmlUtil.java
@@ -150,4 +150,7 @@
 			}
-			builder.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); // https://semgrep.dev/docs/cheat-sheets/java-xxe
-			sDoc = builder.build(reader);
+			builder.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); // https://semgrep.dev/docs/cheat-sheets/java-xxe
+			builder.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+			builder.setFeature("http://xml.org/sax/features/external-general-entities", false);
+			builder.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+			sDoc = builder.build(reader);
 			// ----- Element root = null;
EOF
@@ -150,4 +150,7 @@
}
builder.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); // https://semgrep.dev/docs/cheat-sheets/java-xxe
sDoc = builder.build(reader);
builder.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); // https://semgrep.dev/docs/cheat-sheets/java-xxe
builder.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
builder.setFeature("http://xml.org/sax/features/external-general-entities", false);
builder.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
sDoc = builder.build(reader);
// ----- Element root = null;
Copilot is powered by AI and may make mistakes. Always verify output.

@Ezequiel-Valencia Ezequiel-Valencia May 15, 2025

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.

Secure processing is the strictest possible limitations that can be set due to external validation that is required for SBML.

@Ezequiel-Valencia
Make Every Builder Disable Doctype
04d06e8 
Different Method For Secure Processing
@Ezequiel-Valencia Ezequiel-Valencia force-pushed the save-biomodel-endpoint branch from 20779d6 to 04d06e8 Compare May 15, 2025 13:09
@Ezequiel-Valencia Ezequiel-Valencia merged commit 642dbbb into master May 15, 2025
@Ezequiel-Valencia Ezequiel-Valencia deleted the save-biomodel-endpoint branch May 15, 2025 13:53
@jcschaff jcschaff mentioned this pull request Dec 1, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jcschaff jcschaff jcschaff approved these changes

Assignees

@Ezequiel-Valencia Ezequiel-Valencia

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement BioModel Endpoints

3 participants

@Ezequiel-Valencia @jcschaff @github-advanced-security