Skip to content

fix: extract _check_open_perms, fix dead elsif in __sysopen #337

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
toddr-bot wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: extract _check_open_perms, fix dead elsif in __sysopen #337

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
103 changes: 42 additions & 61 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,39 @@ sub _check_parent_perms {
return _check_perms( $parent_mock, $access );
}

# Shared permission check for open/sysopen paths.
# $is_new: true if the file is being created (contents were undef before O_CREAT).
# $func_name: 'open' or 'sysopen' (for autodie).
# $rw: read/write mode string (e.g. 'r', 'w', 'rw', 'wa').
# Returns 1 if allowed, 0 if denied (sets $! = EACCES and throws if autodie).
sub _check_open_perms {
my ( $mock_file, $abs_path, $rw, $is_new, $func_name, @orig_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, @orig_args );
return 0;
}
}
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, @orig_args );
return 0;
}
}

return 1;
}

my @_tmf_callers;

# Packages where autodie was active when T::MF was imported.
Expand Down Expand Up @@ -2815,27 +2848,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 @@ -3166,26 +3179,8 @@ sub __open (*;$@) {
$rw .= 'w' if grep { $_ eq $mode } qw/+< +> +>> > >>/;
$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;
}
}
}
# Permission check (GH #3)
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 @@ -3326,8 +3321,12 @@ sub __sysopen (*$$;$) {
return undef;
}

# Track whether this is a new file BEFORE O_CREAT populates contents.
# Used by _check_open_perms to decide file-perms vs parent-dir-perms.
my $is_new = !defined $mock_file->{'contents'};

# O_CREAT — POSIX open(2): creating a new file sets atime, mtime, and ctime.
if ( $sysopen_mode & O_CREAT && !defined $mock_file->{'contents'} ) {
if ( $sysopen_mode & O_CREAT && $is_new ) {
$mock_file->{'contents'} = '';
my $now = time;
$mock_file->{'atime'} = $now;
Expand Down Expand Up @@ -3371,25 +3370,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
24 changes: 24 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,28 @@ subtest 'open > on new file checks parent directory perms' => sub {
} 1000, 1000;
};

# =========================================================================
# sysopen O_CREAT on new file checks parent perms (GH #329)
# Before the fix, the elsif branch was dead code — O_CREAT populated
# contents before the perm check, so the parent-dir check never fired.
# =========================================================================

subtest 'sysopen O_CREAT on new file checks parent directory perms (GH #329)' => 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