Skip to content

fix: extract _check_open_perms and fix dead elsif in __sysopen #339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Koan-Bot wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: extract _check_open_perms and fix dead elsif in __sysopen #339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 38 additions & 59 deletions lib/Test/MockFile.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,38 @@ sub _check_parent_perms {
return _check_perms( $parent_mock, $access );
}

# _check_open_perms($mock_file, $abs_path, $rw, $is_new, $func_name, @caller_args)
# Consolidated permission check for __open, __sysopen, and _io_file_mock_open.
# $is_new: true when the file is being created (contents were undef before O_CREAT).
# Returns 1 if allowed, undef if denied (with $! and autodie set).
sub _check_open_perms {
my ( $mock_file, $abs_path, $rw, $is_new, $func_name, @caller_args ) = @_;

return 1 unless defined $_mock_uid;

if ( !$is_new ) {
# Existing file: check file permissions
my $need = 0;
$need |= 4 if $rw =~ /r/;
$need |= 2 if $rw =~ /w/;
if ( !_check_perms( $mock_file, $need ) ) {
$! = EACCES;
_maybe_throw_autodie( $func_name, @caller_args );
return undef;
}
}
elsif ( $rw =~ /w/ ) {
# Creating new file: check parent dir write+execute
if ( !_check_parent_perms( $abs_path, 2 | 1 ) ) {
$! = EACCES;
_maybe_throw_autodie( $func_name, @caller_args );
return undef;
}
}

return 1;
}

my @_tmf_callers;

# Packages where autodie was active when T::MF was imported.
Expand Down Expand Up @@ -2815,27 +2847,7 @@ sub _io_file_mock_open {
$rw .= 'a' if grep { $_ eq $mode } qw/>> +>>/;

# Permission check (GH #3)
if ( defined $_mock_uid ) {
if ( defined $contents ) {
# Existing file: check file permissions
my $need = 0;
$need |= 4 if $rw =~ /r/;
$need |= 2 if $rw =~ /w/;
if ( !_check_perms( $mock_file, $need ) ) {
$! = EACCES;
_maybe_throw_autodie( 'open', @_ );
return undef;
}
}
elsif ( $rw =~ /w/ ) {
# Creating new file: check parent dir write+execute
if ( !_check_parent_perms( $abs_path, 2 | 1 ) ) {
$! = EACCES;
_maybe_throw_autodie( 'open', @_ );
return undef;
}
}
}
return undef unless _check_open_perms( $mock_file, $abs_path, $rw, !defined $contents, 'open', @_ );

# Tie the existing IO::File glob directly (don't create a new one)
tie *{$fh}, 'Test::MockFile::FileHandle', $abs_path, $rw;
Expand Down Expand Up @@ -3167,25 +3179,7 @@ sub __open (*;$@) {
$rw .= 'a' if grep { $_ eq $mode } qw/>> +>>/;

# Permission check (GH #3) — IO::File path must match __open
if ( defined $_mock_uid ) {
if ( defined $contents ) {
my $need = 0;
$need |= 4 if $rw =~ /r/;
$need |= 2 if $rw =~ /w/;
if ( !_check_perms( $mock_file, $need ) ) {
$! = EACCES;
_maybe_throw_autodie( 'open', @_ );
return undef;
}
}
elsif ( $rw =~ /w/ ) {
if ( !_check_parent_perms( $abs_path, 2 | 1 ) ) {
$! = EACCES;
_maybe_throw_autodie( 'open', @_ );
return undef;
}
}
}
return undef unless _check_open_perms( $mock_file, $abs_path, $rw, !defined $contents, 'open', @_ );

my $filefh = IO::File->new;
tie *{$filefh}, 'Test::MockFile::FileHandle', $abs_path, $rw;
Expand Down Expand Up @@ -3319,6 +3313,9 @@ sub __sysopen (*$$;$) {
return undef;
}

# Track whether this is a new file creation before O_CREAT populates contents.
my $is_new = !defined $mock_file->{'contents'};

# O_EXCL
if ( $sysopen_mode & O_EXCL && $sysopen_mode & O_CREAT && defined $mock_file->{'contents'} ) {
$! = EEXIST;
Expand Down Expand Up @@ -3371,25 +3368,7 @@ sub __sysopen (*$$;$) {
}

# Permission check (GH #3)
if ( defined $_mock_uid ) {
if ( defined $mock_file->{'contents'} ) {
my $need = 0;
$need |= 4 if $rw =~ /r/;
$need |= 2 if $rw =~ /w/;
if ( !_check_perms( $mock_file, $need ) ) {
$! = EACCES;
_maybe_throw_autodie( 'sysopen', @_ );
return undef;
}
}
elsif ( $rw =~ /w/ ) {
if ( !_check_parent_perms( $mock_file->{'path'}, 2 | 1 ) ) {
$! = EACCES;
_maybe_throw_autodie( 'sysopen', @_ );
return undef;
}
}
}
return undef unless _check_open_perms( $mock_file, $mock_file->{'path'}, $rw, $is_new, 'sysopen', @_ );

$abs_path //= $mock_file->{'path'};

Expand Down
22 changes: 22 additions & 0 deletions t/perms.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,4 +336,26 @@ subtest 'open > on new file checks parent directory perms' => sub {
} 1000, 1000;
};

# =========================================================================
# sysopen with O_CREAT on new file checks parent perms (GH #329)
# =========================================================================

subtest 'sysopen O_CREAT on new file checks parent directory perms' => sub {
my $parent = Test::MockFile->new_dir( '/perms/sdir', { mode => 0555, uid => 1000, gid => 1000 } );
my $child = Test::MockFile->file('/perms/sdir/newfile');

with_user {
ok( !sysopen( my $fh, '/perms/sdir/newfile', O_WRONLY | O_CREAT ), 'sysopen O_CREAT fails in read-only parent dir' );
is( $! + 0, EACCES, 'errno is EACCES' );
} 1000, 1000;

my $parent2 = Test::MockFile->new_dir( '/perms/sdir2', { mode => 0755, uid => 1000, gid => 1000 } );
my $child2 = Test::MockFile->file('/perms/sdir2/newfile2');

with_user {
ok( sysopen( my $fh, '/perms/sdir2/newfile2', O_WRONLY | O_CREAT ), 'sysopen O_CREAT succeeds in writable parent dir' );
close $fh if $fh;
} 1000, 1000;
};

done_testing();
Loading