Skip to content

[pull] master from asbru-cm:master#92

Open
pull[bot] wants to merge 859 commits into
RogueScholar:masterfrom
asbru-cm:master
Open

[pull] master from asbru-cm:master#92
pull[bot] wants to merge 859 commits into
RogueScholar:masterfrom
asbru-cm:master

Conversation

@pull

@pull pull Bot commented Mar 4, 2020

Copy link
Copy Markdown

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

@pull pull Bot added the ⤵️ pull label Mar 4, 2020
@pull pull Bot added the merge-conflict Resolve conflicts manually label Mar 4, 2020
KlaasT and others added 27 commits November 7, 2020 14:08
Resizing logo
Signed-off-by: Syco <alberto.rinaudo+dino@gmail.com>
- ncat is not a final package, should be nmap
- if string has utf8 characters, text will be copied incompletely because string length was not calculated correctly
When clicking on the tray icon, you have to click twice to untray Ásbrú.
This fix will check that Ásbrú is the active window and force a hide/show operation to make sure it is shown after a single click.
This can be particularly useful when one has all SSH connections defined in ~/.ssh/config
Ubuntu Eoan is EOL since July 17, 2020.
Co-authored-by: Syco <alberto.rinaudo+dino@gmail.com>
- Add new button in connection network settings
- Use url field to fill connection details
Comment thread lib/method/PACMethod_ssh.pm
Comment thread lib/PACKeyBindings.pm
Comment thread lib/PACKeePass.pm
my $selection = $w{window}{gui}{treelist}->get_selection();
my $model = $w{window}{gui}{treelist}->get_model();
my @paths = _getSelectedRows($selection);
$entry = $model->get_value($model->get_iter($paths[0]),0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: In listEntries, clicking 'OK' without a selection causes a crash because the code attempts to access the first element of an empty array, leading to an undef reference.
Severity: CRITICAL

Suggested Fix

Before attempting to get a value from the selection on line 429, add a guard to check if the @paths array contains any elements. If it is empty, return from the subroutine to prevent the crash. For example: return unless @paths;.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: lib/PACKeePass.pm#L429

Potential issue: In the `listEntries` method, when a user clicks the 'OK' button in the
KeePassXC search dialog without selecting an entry, the application will crash. The code
retrieves selected rows into an array `@paths`. If no row is selected, this array is
empty. The subsequent code attempts to access `$paths[0]`, which evaluates to `undef`.
This `undef` value is then passed to `$model->get_iter()`, causing a runtime error due
to an attempt to dereference an undefined value.

Comment on lines +845 to +855
}
}
});
push(@menu_items, {
label => 'KeePassX',
stockicon => 'pac-keepass',
submenu => [
{
label => 'KeePassX title values',
submenu => \@titles
}, {
label => 'KeePassX username values',
submenu => \@usernames
}, {
label => 'KeePassX URL values',
submenu => \@urls
}, {
label => "KeePass Extended Query",
tooltip => "This allows you to select the value to be returned, based on another value's match againt a Perl Regular Expression",
code => sub {$w{send}->insert_text("<KPXRE_GET_(title|username|password|url)_WHERE_(title|username|password|url)==Your_RegExp_here==>", -1, $w{send}->get_position);}
label => 'Add Password KeePassXC',
tooltip => 'KeePassXC Password',
code => sub {
my $pos = $w{send}->get_property('cursor_position');
my $selection = $PACMain::FUNCS{_KEEPASS}->listEntries($$self{_WINDOWEDIT});
if ($selection) {
$w{send}->insert_text("<password|$selection>", -1, $w{expect}->get_position);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: When using the KeePassXC menu to insert a username or password, the text is inserted at the wrong position because the cursor position is read from the expect field instead of the send field.
Severity: MEDIUM

Suggested Fix

In the insert_text calls, replace $w{expect}->get_position with a call that gets the cursor position from the $w{send} widget, such as $w{send}->get_position() or by using the existing but unused $pos variable.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: lib/edit/PACExpectEntry.pm#L843-L855

Potential issue: In the KeePassXC menu item callbacks, text is inserted into the
`$w{send}` widget. However, the insertion position is incorrectly determined by calling
`$w{expect}->get_position()` instead of using the cursor position from the `$w{send}`
widget. This results in text (e.g., `<username|$selection>`) being inserted at the
cursor's location within the "Expect" field, not the "Send" field where the user
intended. This creates a confusing user experience, as text appears in the wrong place.
The presence of an unused variable `$pos` that correctly captures the `$w{send}` cursor
position indicates this was likely an oversight.

Comment thread lib/edit/PACExpectEntry.pm
Comment thread lib/PACKeePass.pm
} else {
$cfg = $self->get_cfg();
}
if (version->parse($$self{kpxc_version}) >= version->parse("2.7.0")) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The application crashes when parsing the KeePassXC version if keepassxc-cli is not installed, as version->parse() is called with an empty string, causing an unhandled exception.
Severity: HIGH

Suggested Fix

Add a guard to lib/PACKeePass.pm at line 542 to ensure $$self{kpxc_version} is not empty before calling version->parse(). For example: if ($$self{kpxc_version} && version->parse($$self{kpxc_version}) >= version->parse("2.7.0")) { ... }.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: lib/PACKeePass.pm#L542

Potential issue: The function `_locateEntries` calls
`version->parse($$self{kpxc_version})` without first validating that
`$$self{kpxc_version}` is a non-empty string. If the `keepassxc-cli` binary is not found
or fails its version check, `$$self{kpxc_version}` remains an empty string. The
`version->parse` method will throw an unhandled exception when called with an empty
string, causing the application to crash. This can be triggered by opening the KeePassXC
search dialog when KeePassXC is not properly configured.

Comment thread lib/PACKeePass.pm
$cfg = $self->get_cfg();
}
$w{var}->set_editable(0);
$ts = stat($$cfg{database})->mtime;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The application crashes when calling stat() on a non-existent KeePass database file because the code attempts to call a method on the resulting undef value.
Severity: CRITICAL

Suggested Fix

Add a check to validate the return value of stat($$cfg{database}) before attempting to call the mtime method on it. If stat() returns undef, handle the error gracefully instead of proceeding.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: lib/PACKeePass.pm#L850

Potential issue: In the `_hasCacheValue` subroutine, the code at line 850 executes `$ts
= stat($$cfg{database})->mtime;` without checking the return value of `stat()`. If a
user has configured an invalid or non-existent path for the KeePass database file,
`stat()` will return `undef`. Attempting to call the `mtime` method on `undef` results
in an unhandled exception, which crashes the application. This occurs when the
application tries to resolve a KeePass mask (e.g., `<username|entry_name>`) with an
invalid database configuration.

Comment thread lib/edit/PACMethod.pm
$$self{container}->foreach(sub {$_[0]->destroy;});
$METHODS{$$self{_METHOD}}->_buildGUI();
$METHODS{$$self{_METHOD}}->update($$self{_CFG}{'options'});
$METHODS{$$self{_METHOD}}->update($$self{_CFG}{'options'}, $$self{_CFG}{"$$self{_METHOD} options"});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The change() method uses a dynamic config key like "SSH options" which doesn't exist, while update() uses the correct 'connection options' key, causing incorrect option handling.
Severity: MEDIUM

Suggested Fix

Unify the configuration key access. In lib/edit/PACMethod.pm at line 142, replace the dynamic key $$self{_CFG}{"$$self{_METHOD} options"} with the correct, static key $$self{_CFG}{'connection options'} to match the usage in the update() method.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: lib/edit/PACMethod.pm#L142

Potential issue: In `lib/edit/PACMethod.pm`, the `change()` method at line 142
constructs a dynamic configuration key (e.g., `"SSH options"`) using
`$$self{_CFG}{"$$self{_METHOD} options"}`. However, this key does not exist in the
configuration. The `update()` method at line 160 correctly uses the key `'connection
options'`. When a user switches connection methods in the UI, `change()` is called and
passes `undef` to the new method's `update()` function. This prevents method-specific
options from being correctly initialized, leading to a loss of configuration settings
during the switch.

KlaasT added 2 commits April 10, 2026 12:09
Filter for only AppImage .deb and .rpm
Delete old snapshot release assets to prevent errors.
Comment thread lib/PACKeePass.pm
Comment on lines +771 to +774
if (ref($self) eq 'PACKeePass') {
# Ignore this call
if (!$$self{frame}{cbUseKeePass}->is_visible()) {
return 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: A crash occurs in _setCapabilities when PACKeePass is initialized without a GUI, as it attempts to call a method on the uninitialized $$self{frame} property.
Severity: CRITICAL

Suggested Fix

In _setCapabilities, add a guard condition to check for the existence of $$self{'frame'} before attempting to access any of its child UI elements. For example: if ($$self{'frame'}) { ... }.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: lib/PACKeePass.pm#L771-L774

Potential issue: The `_setCapabilities` method is called from the `PACKeePass`
constructor. When the constructor is invoked with `$buildgui = 0`, as is the case in
several production code paths like `asbru_conn`, the GUI is not built and
`$$self{frame}` remains `undef`. The code then attempts to call the `is_visible()`
method on `$$self{frame}{cbUseKeePass}`, which is an undefined value. This results in a
crash when initializing `PACKeePass` without a graphical interface.

Comment thread lib/edit/PACExpectEntry.pm
Comment on lines +232 to +239
if ($w{sbUDPPort}->get_chars(0, -1) ge $w{sbUDPPort2}->get_chars(0, -1)) {
$w{sbUDPPort2}->set_value(int($w{sbUDPPort}->get_chars(0, -1)) + 1);
}
});
$w{sbUDPPort2}->signal_connect('value-changed' => sub {
if ($w{sbUDPPort}->get_chars(0, -1) ge $w{sbUDPPort2}->get_chars(0, -1)) {
$w{sbUDPPort}->set_value(int($w{sbUDPPort2}->get_chars(0, -1)) - 1);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The UDP port validation uses string comparison (ge) instead of numeric comparison (>=), which can cause an infinite loop and UI freeze when port numbers have different digit lengths.
Severity: HIGH

Suggested Fix

Replace the string comparison operator ge with the numeric comparison operator >= at line 232 in lib/method/PACMethod_mosh.pm to ensure port numbers are compared numerically.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: lib/method/PACMethod_mosh.pm#L232-L239

Potential issue: In `PACMethod_mosh.pm`, the start and end UDP port values are compared
using a string comparison operator (`ge`) instead of a numeric one (`>=`). This leads to
incorrect logic when the port numbers have a different number of digits. For example, a
start port of `9` is considered greater than an end port of `60000`. This incorrect
comparison can trigger an infinite loop between the `value-changed` signal handlers for
the two port input fields, causing the UI to become unresponsive or crash.

Comment thread utils/pac2asbru.pl
if ($VERBOSE) {
print " - Rename screenshots\n";
}
opendir($ss, "$new_dir/screenshots");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The migration scripts pac2asbru.pl and asbru2pac.pl will crash if the screenshots directory does not exist because they fail to check the return value of opendir.
Severity: CRITICAL

Suggested Fix

Wrap the opendir call and the subsequent while loop in a conditional block to ensure the code only attempts to read the directory if it was successfully opened. For example: if (opendir($ss, "$new_dir/screenshots")) { ... }.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: utils/pac2asbru.pl#L134

Potential issue: The migration scripts `pac2asbru.pl` and `asbru2pac.pl` attempt to open
a `screenshots` directory using `opendir` without checking if the call was successful.
If a user's configuration does not contain a `screenshots` directory, `opendir` will
fail, and the subsequent call to `readdir` on the invalid filehandle will cause a fatal
runtime error. This will abort the entire migration process, preventing users from
successfully migrating their configuration data.

@@ -148,15 +160,20 @@ sub _parseCfgToOptions
$hash{useCompression} = 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The updated split pattern in _parseCfgToOptions incorrectly parses SFTP options, causing settings like SSH version (-1) and compression (-C) to be lost when loading a connection.
Severity: HIGH

Suggested Fix

The split logic in _parseCfgToOptions should be revised to correctly handle both single-character flags (like -1, -C) and key-value options (-o "Key=Value"). One approach is to first extract all -o options with a regex, process them, and then parse the remaining flags from the string.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: lib/method/PACMethod_sftp.pm#L160

Potential issue: In `PACMethod_sftp.pm`, the logic for parsing SFTP options in
`_parseCfgToOptions` was changed to split the command line string on `\s+-o `. This
change prevents single-character flags that are not preceded by `-o`, such as `-1` (for
SSH version 1) and `-C` (for compression), from being parsed correctly. When a
configuration containing these flags is loaded, they are grouped with other options,
causing the parsing logic to fail to identify them. This results in the user's saved
settings for SSH version and compression being silently discarded and reset to default
values.

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

Labels

⤵️ pull merge-conflict Resolve conflicts manually

Projects

None yet

Development

Successfully merging this pull request may close these issues.