Skip to content

N°9909 - Fix setup token unexpectedly deleted during setup due to con… - #996

Open
eespie wants to merge 1 commit into
developfrom
issue/setup_problem
Open

N°9909 - Fix setup token unexpectedly deleted during setup due to con…#996
eespie wants to merge 1 commit into
developfrom
issue/setup_problem

Conversation

@eespie

@eespie eespie commented Aug 11, 2026

Copy link
Copy Markdown
Member

Avoid cleaning the setup token on disk if not owner

Copilot AI balanced review requested due to automatic review settings August 11, 2026 15:05
@CombodoApplicationsAccount CombodoApplicationsAccount added the internal Work made by Combodo label Aug 11, 2026
@greptile-apps

greptile-apps Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR prevents one setup session from deleting the setup token owned by another session.

  • Gates deletion of data/setup/authent on the session token matching the on-disk token.
  • Clears the session token only when that ownership check succeeds.

Confidence Score: 5/5

The PR appears safe to merge because setup-token deletion is now restricted to the session that owns the on-disk token.

The validity check compares the server-side session token with the fixed on-disk token before cleanup, while invalid retained session values remain inert because authorization always revalidates them against the file.

Important Files Changed

Filename Overview
setup/setuputils.class.inc.php Adds an ownership check before deleting the shared setup token; no actionable regression was identified.

Reviews (1): Last reviewed commit: "N°9909 - Fix setup token unexpectedly de..." | Re-trigger Greptile

Copilot AI left a comment

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.

Pull request overview

Protects active setup tokens from deletion by non-owner sessions.

Changes:

  • Validates session ownership before deleting the setup token.
  • Moves session-token cleanup into the ownership branch.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +2180 to +2183
if (self::IsSessionSetupTokenValid()) {
$sTokenFile = utils::GetDataPath().'setup/authent';
if (is_file($sTokenFile)) {
unlink($sTokenFile);
Comment on lines +2180 to 2186
if (self::IsSessionSetupTokenValid()) {
$sTokenFile = utils::GetDataPath().'setup/authent';
if (is_file($sTokenFile)) {
unlink($sTokenFile);
}
Session::Unset('setup_token');
}
$sTokenFile = utils::GetDataPath().'setup/authent';
if (is_file($sTokenFile)) {
unlink($sTokenFile);
if (self::IsSessionSetupTokenValid()) {
@Molkobain
Molkobain requested review from Lenaick and steffunky August 11, 2026 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Work made by Combodo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants