diff --git a/frontend/src/Settings/MediaManagement/MediaManagement.js b/frontend/src/Settings/MediaManagement/MediaManagement.js index 15ce21b84..7a50f4741 100644 --- a/frontend/src/Settings/MediaManagement/MediaManagement.js +++ b/frontend/src/Settings/MediaManagement/MediaManagement.js @@ -372,7 +372,7 @@ class MediaManagement extends Component { - - - Folder chmod mode - - - - - - chown User - - - - - - chown Group - - - } diff --git a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs index 278d73ee1..d9cc1ae20 100644 --- a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs +++ b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs @@ -1048,7 +1048,7 @@ private void WithEmulatedDiskProvider() .Returns(new List()); Mocker.GetMock() - .Setup(v => v.CopyPermissions(It.IsAny(), It.IsAny(), false)); + .Setup(v => v.CopyPermissions(It.IsAny(), It.IsAny())); } private void WithRealDiskProvider() @@ -1112,7 +1112,7 @@ private void WithRealDiskProvider() .Returns(s => new FileStream(s, FileMode.Open, FileAccess.Read)); Mocker.GetMock() - .Setup(v => v.CopyPermissions(It.IsAny(), It.IsAny(), false)); + .Setup(v => v.CopyPermissions(It.IsAny(), It.IsAny())); } private void WithMockMount(string root) diff --git a/src/NzbDrone.Common/Disk/DiskProviderBase.cs b/src/NzbDrone.Common/Disk/DiskProviderBase.cs index b23350dd4..6e43ad16e 100644 --- a/src/NzbDrone.Common/Disk/DiskProviderBase.cs +++ b/src/NzbDrone.Common/Disk/DiskProviderBase.cs @@ -38,8 +38,8 @@ public static StringComparison PathStringComparison public abstract long? GetAvailableSpace(string path); public abstract void InheritFolderPermissions(string filename); - public abstract void SetPermissions(string path, string mask, string user, string group); - public abstract void CopyPermissions(string sourcePath, string targetPath, bool includeOwner); + public abstract void SetPermissions(string path, string mask); + public abstract void CopyPermissions(string sourcePath, string targetPath); public abstract long? GetTotalSize(string path); public DateTime FolderGetCreationTime(string path) @@ -543,5 +543,10 @@ public void SaveStream(Stream stream, string path) stream.CopyTo(fileStream); } } + + public virtual bool IsValidFilePermissionMask(string mask) + { + throw new NotSupportedException(); + } } } diff --git a/src/NzbDrone.Common/Disk/IDiskProvider.cs b/src/NzbDrone.Common/Disk/IDiskProvider.cs index 2fe2f6359..ef1628888 100644 --- a/src/NzbDrone.Common/Disk/IDiskProvider.cs +++ b/src/NzbDrone.Common/Disk/IDiskProvider.cs @@ -11,8 +11,8 @@ public interface IDiskProvider { long? GetAvailableSpace(string path); void InheritFolderPermissions(string filename); - void SetPermissions(string path, string mask, string user, string group); - void CopyPermissions(string sourcePath, string targetPath, bool includeOwner = false); + void SetPermissions(string path, string mask); + void CopyPermissions(string sourcePath, string targetPath); long? GetTotalSize(string path); DateTime FolderGetCreationTime(string path); DateTime FolderGetLastWrite(string path); @@ -55,5 +55,6 @@ public interface IDiskProvider List GetFileInfos(string path, SearchOption searchOption = SearchOption.TopDirectoryOnly); void RemoveEmptySubfolders(string path); void SaveStream(Stream stream, string path); + bool IsValidFilePermissionMask(string mask); } } diff --git a/src/NzbDrone.Core/Configuration/ConfigService.cs b/src/NzbDrone.Core/Configuration/ConfigService.cs index fce21260c..d16040161 100644 --- a/src/NzbDrone.Core/Configuration/ConfigService.cs +++ b/src/NzbDrone.Core/Configuration/ConfigService.cs @@ -262,27 +262,6 @@ public string FileChmod set { SetValue("FileChmod", value); } } - public string FolderChmod - { - get { return GetValue("FolderChmod", "0755"); } - - set { SetValue("FolderChmod", value); } - } - - public string ChownUser - { - get { return GetValue("ChownUser", ""); } - - set { SetValue("ChownUser", value); } - } - - public string ChownGroup - { - get { return GetValue("ChownGroup", ""); } - - set { SetValue("ChownGroup", value); } - } - public string MetadataSource { get { return GetValue("MetadataSource", ""); } diff --git a/src/NzbDrone.Core/Configuration/IConfigService.cs b/src/NzbDrone.Core/Configuration/IConfigService.cs index ef7afcc91..4d0debfe4 100644 --- a/src/NzbDrone.Core/Configuration/IConfigService.cs +++ b/src/NzbDrone.Core/Configuration/IConfigService.cs @@ -43,9 +43,6 @@ public interface IConfigService //Permissions (Media Management) bool SetPermissionsLinux { get; set; } string FileChmod { get; set; } - string FolderChmod { get; set; } - string ChownUser { get; set; } - string ChownGroup { get; set; } //Indexers int Retention { get; set; } diff --git a/src/NzbDrone.Core/Datastore/Migration/006_remove_chown_and_folderchmod_config.cs b/src/NzbDrone.Core/Datastore/Migration/006_remove_chown_and_folderchmod_config.cs new file mode 100644 index 000000000..3db6c0f7a --- /dev/null +++ b/src/NzbDrone.Core/Datastore/Migration/006_remove_chown_and_folderchmod_config.cs @@ -0,0 +1,14 @@ +using FluentMigrator; +using NzbDrone.Core.Datastore.Migration.Framework; + +namespace NzbDrone.Core.Datastore.Migration +{ + [Migration(006)] + public class remove_chown_and_folderchmod_config : NzbDroneMigrationBase + { + protected override void MainDbUpgrade() + { + Execute.Sql("DELETE FROM config WHERE Key IN ('folderchmod', 'chownuser')"); + } + } +} diff --git a/src/NzbDrone.Core/MediaFiles/DiskScanService.cs b/src/NzbDrone.Core/MediaFiles/DiskScanService.cs index 55364231c..874ee251f 100644 --- a/src/NzbDrone.Core/MediaFiles/DiskScanService.cs +++ b/src/NzbDrone.Core/MediaFiles/DiskScanService.cs @@ -14,6 +14,7 @@ using NzbDrone.Core.Books; using NzbDrone.Core.Books.Calibre; using NzbDrone.Core.MediaFiles.BookImport; +using NzbDrone.Core.Configuration; using NzbDrone.Core.MediaFiles.Commands; using NzbDrone.Core.MediaFiles.Events; using NzbDrone.Core.Messaging.Commands; @@ -39,6 +40,7 @@ public class DiskScanService : public static readonly Regex ExcludedSubFoldersRegex = new Regex(@"(?:\\|\/|^)(?:extras|@eadir|extrafanart|plex versions|\.[^\\/]+)(?:\\|\/)", RegexOptions.Compiled | RegexOptions.IgnoreCase); public static readonly Regex ExcludedFilesRegex = new Regex(@"^\._|^Thumbs\.db$|^\.DS_store$|\.partial~$", RegexOptions.Compiled | RegexOptions.IgnoreCase); + private readonly IConfigService _configService; private readonly IDiskProvider _diskProvider; private readonly ICalibreProxy _calibre; private readonly IMediaFileService _mediaFileService; @@ -50,7 +52,8 @@ public class DiskScanService : private readonly IEventAggregator _eventAggregator; private readonly Logger _logger; - public DiskScanService(IDiskProvider diskProvider, + public DiskScanService(IConfigService configService, + IDiskProvider diskProvider, ICalibreProxy calibre, IMediaFileService mediaFileService, IMakeImportDecision importDecisionMaker, @@ -61,6 +64,7 @@ public DiskScanService(IDiskProvider diskProvider, IEventAggregator eventAggregator, Logger logger) { + _configService = configService; _diskProvider = diskProvider; _calibre = calibre; diff --git a/src/NzbDrone.Core/MediaFiles/MediaFileAttributeService.cs b/src/NzbDrone.Core/MediaFiles/MediaFileAttributeService.cs index 5591bb599..cb9097453 100644 --- a/src/NzbDrone.Core/MediaFiles/MediaFileAttributeService.cs +++ b/src/NzbDrone.Core/MediaFiles/MediaFileAttributeService.cs @@ -62,7 +62,7 @@ public void SetFolderPermissions(string path) { if (OsInfo.IsNotWindows) { - SetMonoPermissions(path, _configService.FolderChmod); + SetMonoPermissions(path, _configService.FileChmod); } } @@ -84,7 +84,7 @@ private void SetMonoPermissions(string path, string permissions) try { - _diskProvider.SetPermissions(path, permissions, _configService.ChownUser, _configService.ChownGroup); + _diskProvider.SetPermissions(path, permissions); } catch (Exception ex) { diff --git a/src/NzbDrone.Core/Update/InstallUpdateService.cs b/src/NzbDrone.Core/Update/InstallUpdateService.cs index a50976d74..6a905f46c 100644 --- a/src/NzbDrone.Core/Update/InstallUpdateService.cs +++ b/src/NzbDrone.Core/Update/InstallUpdateService.cs @@ -139,7 +139,7 @@ private void InstallUpdate(UpdatePackage updatePackage) // Set executable flag on update app if (OsInfo.IsOsx || (OsInfo.IsLinux && PlatformInfo.IsNetCore)) { - _diskProvider.SetPermissions(_appFolderInfo.GetUpdateClientExePath(updatePackage.Runtime), "0755", null, null); + _diskProvider.SetPermissions(_appFolderInfo.GetUpdateClientExePath(updatePackage.Runtime), "0755"); } _logger.Info("Starting update client {0}", _appFolderInfo.GetUpdateClientExePath(updatePackage.Runtime)); diff --git a/src/NzbDrone.Core/Validation/FileChmodValidator.cs b/src/NzbDrone.Core/Validation/FileChmodValidator.cs new file mode 100644 index 000000000..c9f0881a7 --- /dev/null +++ b/src/NzbDrone.Core/Validation/FileChmodValidator.cs @@ -0,0 +1,26 @@ +using FluentValidation.Validators; +using NzbDrone.Common.Disk; + +namespace NzbDrone.Core.Validation +{ + public class FileChmodValidator : PropertyValidator + { + private readonly IDiskProvider _diskProvider; + + public FileChmodValidator(IDiskProvider diskProvider) + : base("Must contain a valid Unix permissions octal") + { + _diskProvider = diskProvider; + } + + protected override bool IsValid(PropertyValidatorContext context) + { + if (context.PropertyValue == null) + { + return false; + } + + return _diskProvider.IsValidFilePermissionMask(context.PropertyValue.ToString()); + } + } +} diff --git a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs index 585c19eb4..19f008757 100644 --- a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs +++ b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs @@ -151,11 +151,100 @@ public void should_copy_folder_permissions() Syscall.stat(dst, out var dstStat); dstStat.st_mode.Should().Be(origStat.st_mode); - Subject.CopyPermissions(src, dst, false); + Subject.CopyPermissions(src, dst); // Verify CopyPermissions Syscall.stat(dst, out dstStat); dstStat.st_mode.Should().Be(srcStat.st_mode); } + + [Test] + public void should_set_file_permissions() + { + var tempFile = GetTempFilePath(); + + File.WriteAllText(tempFile, "File1"); + SetWritePermissions(tempFile, false); + + // Verify test setup + Syscall.stat(tempFile, out var fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0444"); + + Subject.SetPermissions(tempFile, "644"); + Syscall.stat(tempFile, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0644"); + + Subject.SetPermissions(tempFile, "0644"); + Syscall.stat(tempFile, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0644"); + + Subject.SetPermissions(tempFile, "1664"); + Syscall.stat(tempFile, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("1664"); + } + + [Test] + public void should_set_folder_permissions() + { + var tempPath = GetTempFilePath(); + + Directory.CreateDirectory(tempPath); + SetWritePermissions(tempPath, false); + + // Verify test setup + Syscall.stat(tempPath, out var fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0555"); + + Subject.SetPermissions(tempPath, "644"); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0755"); + + Subject.SetPermissions(tempPath, "0644"); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0755"); + + Subject.SetPermissions(tempPath, "1664"); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("1775"); + + Subject.SetPermissions(tempPath, "775"); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0775"); + + Subject.SetPermissions(tempPath, "640"); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0750"); + + Subject.SetPermissions(tempPath, "0041"); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0051"); + + // reinstate sane permissions so fokder can be cleaned up + Subject.SetPermissions(tempPath, "775"); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0775"); + } + + [Test] + public void IsValidFilePermissionMask_should_return_correct() + { + // Files may not be executable + Subject.IsValidFilePermissionMask("0777").Should().BeFalse(); + Subject.IsValidFilePermissionMask("0544").Should().BeFalse(); + Subject.IsValidFilePermissionMask("0454").Should().BeFalse(); + Subject.IsValidFilePermissionMask("0445").Should().BeFalse(); + + // No special bits should be set + Subject.IsValidFilePermissionMask("1644").Should().BeFalse(); + Subject.IsValidFilePermissionMask("2644").Should().BeFalse(); + Subject.IsValidFilePermissionMask("4644").Should().BeFalse(); + Subject.IsValidFilePermissionMask("7644").Should().BeFalse(); + + // Files should be readable and writeable by owner + Subject.IsValidFilePermissionMask("0400").Should().BeFalse(); + Subject.IsValidFilePermissionMask("0000").Should().BeFalse(); + Subject.IsValidFilePermissionMask("0200").Should().BeFalse(); + Subject.IsValidFilePermissionMask("0600").Should().BeTrue(); + } } } diff --git a/src/NzbDrone.Mono/Disk/DiskProvider.cs b/src/NzbDrone.Mono/Disk/DiskProvider.cs index b09eb4601..bf09fa7e0 100644 --- a/src/NzbDrone.Mono/Disk/DiskProvider.cs +++ b/src/NzbDrone.Mono/Disk/DiskProvider.cs @@ -81,13 +81,62 @@ public override void InheritFolderPermissions(string filename) } } - public override void SetPermissions(string path, string mask, string user, string group) + public override void SetPermissions(string path, string mask) { - SetPermissions(path, mask); - SetOwner(path, user, group); + _logger.Debug("Setting permissions: {0} on {1}", mask, path); + + var permissions = NativeConvert.FromOctalPermissionString(mask); + + if (Directory.Exists(path)) + { + permissions = GetFolderPermissions(permissions); + } + + if (Syscall.chmod(path, permissions) < 0) + { + var error = Stdlib.GetLastError(); + + throw new LinuxPermissionsException("Error setting permissions: " + error); + } } - public override void CopyPermissions(string sourcePath, string targetPath, bool includeOwner) + private static FilePermissions GetFolderPermissions(FilePermissions permissions) + { + permissions |= (FilePermissions)((int)(permissions & (FilePermissions.S_IRUSR | FilePermissions.S_IRGRP | FilePermissions.S_IROTH)) >> 2); + + return permissions; + } + + public override bool IsValidFilePermissionMask(string mask) + { + try + { + var permissions = NativeConvert.FromOctalPermissionString(mask); + + if ((permissions & (FilePermissions.S_ISUID | FilePermissions.S_ISGID | FilePermissions.S_ISVTX)) != 0) + { + return false; + } + + if ((permissions & (FilePermissions.S_IXUSR | FilePermissions.S_IXGRP | FilePermissions.S_IXOTH)) != 0) + { + return false; + } + + if ((permissions & (FilePermissions.S_IRUSR | FilePermissions.S_IWUSR)) != (FilePermissions.S_IRUSR | FilePermissions.S_IWUSR)) + { + return false; + } + + return true; + } + catch (FormatException) + { + return false; + } + } + + public override void CopyPermissions(string sourcePath, string targetPath) { try { @@ -98,11 +147,6 @@ public override void CopyPermissions(string sourcePath, string targetPath, bool { Syscall.chmod(targetPath, srcStat.st_mode); } - - if (includeOwner && (srcStat.st_uid != tgtStat.st_uid || srcStat.st_gid != tgtStat.st_gid)) - { - Syscall.chown(targetPath, srcStat.st_uid, srcStat.st_gid); - } } catch (Exception ex) { @@ -246,39 +290,6 @@ public override bool TryCreateHardLink(string source, string destination) } } - private void SetPermissions(string path, string mask) - { - Logger.Debug("Setting permissions: {0} on {1}", mask, path); - - var filePermissions = NativeConvert.FromOctalPermissionString(mask); - - if (Syscall.chmod(path, filePermissions) < 0) - { - var error = Stdlib.GetLastError(); - - throw new LinuxPermissionsException("Error setting file permissions: " + error); - } - } - - private void SetOwner(string path, string user, string group) - { - if (string.IsNullOrWhiteSpace(user) && string.IsNullOrWhiteSpace(group)) - { - Logger.Debug("User and Group for chown not configured, skipping chown."); - return; - } - - var userId = GetUserId(user); - var groupId = GetGroupId(group); - - if (Syscall.chown(path, userId, groupId) < 0) - { - var error = Stdlib.GetLastError(); - - throw new LinuxPermissionsException("Error setting file owner and/or group: " + error); - } - } - private uint GetUserId(string user) { if (user.IsNullOrWhiteSpace()) diff --git a/src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs b/src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs index a2ea4d2b6..3b87d9d03 100644 --- a/src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs +++ b/src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs @@ -122,7 +122,7 @@ public void Start(string installationFolder, int processId) // Set executable flag on Readarr app if (OsInfo.IsOsx || (OsInfo.IsLinux && PlatformInfo.IsNetCore)) { - _diskProvider.SetPermissions(Path.Combine(installationFolder, "Readarr"), "0755", null, null); + _diskProvider.SetPermissions(Path.Combine(installationFolder, "Readarr"), "0755"); } } catch (Exception e) diff --git a/src/NzbDrone.Windows/Disk/DiskProvider.cs b/src/NzbDrone.Windows/Disk/DiskProvider.cs index a0a01e86e..eda3b2b8f 100644 --- a/src/NzbDrone.Windows/Disk/DiskProvider.cs +++ b/src/NzbDrone.Windows/Disk/DiskProvider.cs @@ -58,11 +58,11 @@ public override void InheritFolderPermissions(string filename) file.SetAccessControl(fs); } - public override void SetPermissions(string path, string mask, string user, string group) + public override void SetPermissions(string path, string mask) { } - public override void CopyPermissions(string sourcePath, string targetPath, bool includeOwner) + public override void CopyPermissions(string sourcePath, string targetPath) { } diff --git a/src/Readarr.Api.V1/Config/MediaManagementConfigModule.cs b/src/Readarr.Api.V1/Config/MediaManagementConfigModule.cs index a03f56c57..3eaf411fd 100644 --- a/src/Readarr.Api.V1/Config/MediaManagementConfigModule.cs +++ b/src/Readarr.Api.V1/Config/MediaManagementConfigModule.cs @@ -1,17 +1,18 @@ -using FluentValidation; +using FluentValidation; +using NzbDrone.Common.EnvironmentInfo; using NzbDrone.Core.Configuration; +using NzbDrone.Core.Validation; using NzbDrone.Core.Validation.Paths; namespace Readarr.Api.V1.Config { public class MediaManagementConfigModule : ReadarrConfigModule { - public MediaManagementConfigModule(IConfigService configService, PathExistsValidator pathExistsValidator) + public MediaManagementConfigModule(IConfigService configService, PathExistsValidator pathExistsValidator, FileChmodValidator fileChmodValidator) : base(configService) { SharedValidator.RuleFor(c => c.RecycleBinCleanupDays).GreaterThanOrEqualTo(0); - SharedValidator.RuleFor(c => c.FileChmod).NotEmpty(); - SharedValidator.RuleFor(c => c.FolderChmod).NotEmpty(); + SharedValidator.RuleFor(c => c.FileChmod).SetValidator(fileChmodValidator).When(c => !string.IsNullOrEmpty(c.FileChmod) && (OsInfo.IsLinux || OsInfo.IsOsx)); SharedValidator.RuleFor(c => c.RecycleBin).IsValidPath().SetValidator(pathExistsValidator).When(c => !string.IsNullOrWhiteSpace(c.RecycleBin)); SharedValidator.RuleFor(c => c.MinimumFreeSpaceWhenImporting).GreaterThanOrEqualTo(100); } diff --git a/src/Readarr.Api.V1/Config/MediaManagementConfigResource.cs b/src/Readarr.Api.V1/Config/MediaManagementConfigResource.cs index 8e4147493..66f359cbc 100644 --- a/src/Readarr.Api.V1/Config/MediaManagementConfigResource.cs +++ b/src/Readarr.Api.V1/Config/MediaManagementConfigResource.cs @@ -20,9 +20,6 @@ public class MediaManagementConfigResource : RestResource public bool SetPermissionsLinux { get; set; } public string FileChmod { get; set; } - public string FolderChmod { get; set; } - public string ChownUser { get; set; } - public string ChownGroup { get; set; } public bool SkipFreeSpaceCheckWhenImporting { get; set; } public int MinimumFreeSpaceWhenImporting { get; set; } @@ -50,9 +47,6 @@ public static MediaManagementConfigResource ToResource(IConfigService model) SetPermissionsLinux = model.SetPermissionsLinux, FileChmod = model.FileChmod, - FolderChmod = model.FolderChmod, - ChownUser = model.ChownUser, - ChownGroup = model.ChownGroup, SkipFreeSpaceCheckWhenImporting = model.SkipFreeSpaceCheckWhenImporting, MinimumFreeSpaceWhenImporting = model.MinimumFreeSpaceWhenImporting,