Skip to content

Conversation

@DrakiaXYZ
Copy link
Contributor

No description provided.

DrakiaXYZ and others added 30 commits October 26, 2025 23:04
- Switch back from File.Rename to File.Move, as Rename is throwing exceptions on some users systems
- Switch back from File.Rename to File.Move, as Rename is throwing exceptions on some users systems

Co-authored-by: DrakiaXYZ <565558+TheDgtl@users.noreply.github.com>
Validate core assembly reference when loading mods
…ross other maps that have zombie kill quests
`ReplaceBotHostiltiy` has optional map whitelist param
bosses = hostile to player not to pmc bots
followers = hostile to player not to pmc bots
pmcs = hostile to player + always hostile to scavs
scavs = hostile to player and pmc bots
raiders = hostile to player and pmc bots

Adjusted infection rates to just maps with zombie kill quests
Fixed preset typo `bossTagillaAgro`
* Addd a new ReleaseCheckService to notify users of updates
- Pulls the latest release from GitHub API to compare the tag against the users current SPT version
- Runs at the very end of the startup process to avoid being pushed off screen by mod logging
- Only notifies of patch version increments, not major or minor increments
- Links the release notes so users can Ctrl+Click to open directly to the upgrade page
- Is run on its own thread, and discards all errors, so as to not impact users without an internet connection or ability to access GitHub

* Formatting

* Use record for the ReleaseInformation class

---------

Co-authored-by: DrakiaXYZ <565558+TheDgtl@users.noreply.github.com>
Added `ClearProfileData()`
Replaced filepath access with `Path.Combine`
Reduced various sources of duplication
Co-authored-by: Tyfon <29051038+tyfon7@users.noreply.github.com>
ArchangelWTF and others added 9 commits October 31, 2025 16:53
When exiting raid with severe muscle pain, prevent client instructing server to add mild muscle pain
When exiting a raid with effect that has a timer, decrease timer value by amount of time spent in raid
Cleaned up `SaveWeaponBuild` and `SaveEquipmentBuild`
@qodo-merge-for-open-source
Copy link

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: Check Target Branch

Failed stage: Target Branch Check [❌]

Failure summary:

The action failed due to an explicit policy check in the workflow that blocks pull requests
targeting the main branch.
- The job "Check Target Branch" executed a step that echoed an error:
"Pull requests targeting the main branch are not allowed."
- It instructed to change the target
branch to develop or a feature branch.
- The step terminated the job with exit 1, leading to the
workflow failure.

Relevant error logs:
1:  ##[group]Runner Image Provisioner
2:  Hosted Compute Agent
...

11:  ##[group]Runner Image
12:  Image: ubuntu-24.04
13:  Version: 20251030.96.2
14:  Included Software: https://github.com/actions/runner-images/blob/ubuntu24/20251030.96/images/ubuntu/Ubuntu2404-Readme.md
15:  Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu24%2F20251030.96
16:  ##[endgroup]
17:  ##[group]GITHUB_TOKEN Permissions
18:  Contents: read
19:  Metadata: read
20:  Packages: read
21:  ##[endgroup]
22:  Secret source: Actions
23:  Prepare workflow directory
24:  Prepare all required actions
25:  Complete job name: Check Target Branch
26:  ##[group]Run echo "::error::Pull requests targeting the 'main' branch are not allowed."
27:  �[36;1mecho "::error::Pull requests targeting the 'main' branch are not allowed."�[0m
28:  �[36;1mecho "Please change the target branch of this PR to 'develop' or a feature branch."�[0m
29:  �[36;1mexit 1�[0m
30:  shell: /usr/bin/bash -e {0}
31:  ##[endgroup]
32:  ##[error]Pull requests targeting the 'main' branch are not allowed.
33:  Please change the target branch of this PR to 'develop' or a feature branch.
34:  ##[error]Process completed with exit code 1.
35:  Cleaning up orphan processes

@qodo-merge-for-open-source
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Avoid potential null reference exception

Remove the null-forgiving operator ! when getting pmcSortingTable and add a null
check before its use to prevent a potential NullReferenceException.

Libraries/SPTarkov.Server.Core/Migration/Migrations/Fixes/InvalidPocketFix.cs [210]

-var pmcSortingTable = pmcInventory?["sortingTable"]?.GetValue<string>()!;
+var pmcSortingTable = pmcInventory?["sortingTable"]?.GetValue<string>();
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential NullReferenceException due to the unsafe use of the null-forgiving operator !, which could crash the server during profile migration.

Medium
Correct yearly event configuration

For the "Night of The Cult" quest, set the yearly flag to true to align with its
"Halloween" season and ensure it recurs annually as intended.

Libraries/SPTarkov.Server.Assets/SPT_Data/configs/quest.json [175-178]

 "season": "Halloween",
 "startTimestamp": 1341615600000,
 "endTimestamp": "",
-"yearly": false
+"yearly": true
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a logical inconsistency where a "Halloween" seasonal event is not configured to be a yearly recurring event, which could lead to incorrect quest behavior.

Medium
Learned
best practice
Log and dispose on failures

Avoid empty catch blocks; log a concise message on failure and dispose
HttpClient via using to prevent resource leaks. Include minimal context to aid
diagnostics.

Libraries/SPTarkov.Server.Core/Services/ReleaseCheckService.cs [26-63]

 private async Task CheckForUpdate()
 {
     try
     {
-        var httpClient = new HttpClient();
-        ...
+        using var httpClient = new HttpClient();
+        httpClient.DefaultRequestHeaders.UserAgent.TryParseAdd("SP-Tarkov");
+        httpClient.DefaultRequestHeaders.Add("X-GitHub-Api-Version", "2022-11-28");
+
         var release = await httpClient.GetFromJsonAsync<ReleaseInformation>(
             "https://api.github.com/repos/sp-tarkov/build/releases/latest"
         );
-        if (release != null)
+        if (release is null)
         {
-            ...
-            if (latestVersion > currentVersion)
-            {
-                logger.Warning($"A new version of SPT is available! SPT v{release.Version}");
-                logger.Warning($"Released {release.ReleaseDate.ToLocalTime()}");
-                logger.Warning($"Release Notes: {release.DownloadUrl}");
-            }
+            return;
+        }
+
+        Version latestVersion = new(release.Version);
+        Version currentVersion = ProgramStatics.SPT_VERSION();
+        Range currentVersionRange = new($"~{currentVersion.Major}.{currentVersion.Minor}.0");
+
+        if (!currentVersionRange.IsSatisfied(latestVersion))
+        {
+            return;
+        }
+
+        if (latestVersion > currentVersion)
+        {
+            logger.Warning($"A new version of SPT is available! SPT v{release.Version}");
+            logger.Warning($"Released {release.ReleaseDate.ToLocalTime()}");
+            logger.Warning($"Release Notes: {release.DownloadUrl}");
         }
     }
-    // We ignore errors, this isn't critical to run, and we don't want to scare users
-    catch { }
+    catch (Exception ex)
+    {
+        logger.Debug(ex, "Release check failed; continuing without update notification");
+    }
 }
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Guard high-risk state mutations and external calls with robust error handling and clear logging; avoid silently swallowing exceptions.

Low
  • More

@DrakiaXYZ DrakiaXYZ merged commit d2e2f04 into main Oct 31, 2025
4 of 5 checks passed
@qodo-merge-for-open-source
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Unbounded HTTP request

Description: The GitHub release check uses an unauthenticated HttpClient to call the public API without
timeouts or retry/backoff and swallows all exceptions in a broad catch, which could allow
a hang or hide failures; set explicit timeouts and handle exceptions narrowly.
ReleaseCheckService.cs [30-63]

Referred Code
var httpClient = new HttpClient();

// These headers are _required_ by GitHub API
httpClient.DefaultRequestHeaders.UserAgent.TryParseAdd("SP-Tarkov");
httpClient.DefaultRequestHeaders.Add("X-GitHub-Api-Version", "2022-11-28");

// TODO: We could probably throw this into a config somewhere, for now hard code it
var release = await httpClient.GetFromJsonAsync<ReleaseInformation>(
    "https://api.github.com/repos/sp-tarkov/build/releases/latest"
);
if (release != null)
{
    Version latestVersion = new(release.Version);
    Version currentVersion = ProgramStatics.SPT_VERSION();
    Range currentVersionRange = new($"~{currentVersion.Major}.{currentVersion.Minor}.0");

    // First make sure the latest release is in our range, this stops "4.1.0" from being detected as a valid upgrade for "4.0.1"
    if (!currentVersionRange.IsSatisfied(latestVersion))
    {
        return;
    }


 ... (clipped 13 lines)
Non-atomic file write

Description: Replacing File.Replace with File.Move(overwrite:true) during atomic write may risk partial
updates if the move crosses volumes or in case of crash, reducing atomicity guarantees
compared to File.Replace with a backup.
FileUtil.cs [135-139]

Referred Code
    }

    // Overwrite over the old file
    File.Move(tempFilePath, filePath, overwrite: true);
}
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Swallowed errors: The update check wraps the whole operation in a broad try/catch that silently ignores all
exceptions without logging, hiding failures and context.

Referred Code
try
{
    var httpClient = new HttpClient();

    // These headers are _required_ by GitHub API
    httpClient.DefaultRequestHeaders.UserAgent.TryParseAdd("SP-Tarkov");
    httpClient.DefaultRequestHeaders.Add("X-GitHub-Api-Version", "2022-11-28");

    // TODO: We could probably throw this into a config somewhere, for now hard code it
    var release = await httpClient.GetFromJsonAsync<ReleaseInformation>(
        "https://api.github.com/repos/sp-tarkov/build/releases/latest"
    );
    if (release != null)
    {
        Version latestVersion = new(release.Version);
        Version currentVersion = ProgramStatics.SPT_VERSION();
        Range currentVersionRange = new($"~{currentVersion.Major}.{currentVersion.Minor}.0");

        // First make sure the latest release is in our range, this stops "4.1.0" from being detected as a valid upgrade for "4.0.1"
        if (!currentVersionRange.IsSatisfied(latestVersion))
        {


 ... (clipped 15 lines)
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing logging: New functionality performs external update checks without emitting structured audit logs
of the action, inputs, or outcomes, making it hard to reconstruct events.

Referred Code
private async Task CheckForUpdate()
{
    try
    {
        var httpClient = new HttpClient();

        // These headers are _required_ by GitHub API
        httpClient.DefaultRequestHeaders.UserAgent.TryParseAdd("SP-Tarkov");
        httpClient.DefaultRequestHeaders.Add("X-GitHub-Api-Version", "2022-11-28");

        // TODO: We could probably throw this into a config somewhere, for now hard code it
        var release = await httpClient.GetFromJsonAsync<ReleaseInformation>(
            "https://api.github.com/repos/sp-tarkov/build/releases/latest"
        );
        if (release != null)
        {
            Version latestVersion = new(release.Version);
            Version currentVersion = ProgramStatics.SPT_VERSION();
            Range currentVersionRange = new($"~{currentVersion.Major}.{currentVersion.Minor}.0");

            // First make sure the latest release is in our range, this stops "4.1.0" from being detected as a valid upgrade for "4.0.1"


 ... (clipped 16 lines)
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Wipe endpoint risk: The new wipe flow toggles profile wipe based on input without visible
authentication/authorisation checks in the diff, which may be acceptable but requires
confirmation.

Referred Code
public bool Wipe(RegisterData info)
{
    if (!CoreConfig.AllowProfileWipe)
    {
        return false;
    }

    var sessionId = Login(info);

    if (!sessionId)
    {
        var profileInfo = saveServer
            .GetProfiles()
            .FirstOrDefault(x => x.Value.ProfileInfo?.Username == info.Username)
            .Value.ProfileInfo;

        profileInfo!.Edition = info.Edition;
        profileInfo.IsWiped = true;
    }

    return sessionId;


 ... (clipped 2 lines)
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants