From e01eabe5bc8a3dc3676551d64919da626900cb5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C5=8Dan?= Date: Sun, 29 Mar 2026 05:46:48 -0600 Subject: [PATCH] fix: extract _check_open_perms helper and fix dead elsif in __sysopen The permission-check block was copy-pasted three times across __open, _io_file_mock_open, and __sysopen. In __sysopen, the elsif branch (parent-dir write check for new file creation) was unreachable because O_CREAT sets contents before the check runs. Extract _check_open_perms() as a single source of truth. In __sysopen, track $is_new before O_CREAT populates contents so the helper correctly enforces parent-dir permissions on file creation. Closes #329 Closes #330 Co-Authored-By: Claude Opus 4.6 --- lib/Test/MockFile.pm | 97 +++++++++++++++++--------------------------- t/perms.t | 22 ++++++++++ 2 files changed, 60 insertions(+), 59 deletions(-) diff --git a/lib/Test/MockFile.pm b/lib/Test/MockFile.pm index 6c5d4cf..4a5525f 100644 --- a/lib/Test/MockFile.pm +++ b/lib/Test/MockFile.pm @@ -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. @@ -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; @@ -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; @@ -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; @@ -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'}; diff --git a/t/perms.t b/t/perms.t index 664da5f..2497a88 100644 --- a/t/perms.t +++ b/t/perms.t @@ -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();