From 07e06ea2ad3b8413e3f5d586a493128e918039a7 Mon Sep 17 00:00:00 2001 From: Jason Date: Tue, 28 Jul 2026 17:59:45 -0400 Subject: [PATCH 1/2] DYN-10661: never return a null templates directory PathManager.templatesDirectory had no initializer. Its only assignment was inside UpdatePreferenceItemPath, which returns early without assigning when PathHelper.CreateFolderIfNotExist rejects the location (unwritable %ProgramData%, unavailable network share, invalid path). A single failed call at startup therefore left PathManager .TemplatesDirectory null for the remainder of the session. In 3.6.2 that null flowed into path.Contains(TemplatesDirectory) in DynamoViewModel.InternalSaveAs, so String.Contains threw ArgumentNullException and Save, Save As and Ctrl+S all failed. PR #17190 replaced that call with the null-guarded IsPathInTemplateDirectoryTree, which stopped the exception but left the null in place - so the template write-protection added by that PR silently does nothing for affected users. Seed templatesDirectory with defaultTemplatesDirectory when common directories are built, so the property always reports a usable path and a rejected preference falls back instead of nulling out. Log the rejection to the Dynamo log as well; the silent failure is why this went unnoticed since the check was introduced in #14871. Co-Authored-By: Claude Opus 5 (1M context) --- src/DynamoCore/Configuration/PathManager.cs | 7 +++ src/DynamoCore/Models/DynamoModel.cs | 9 ++- .../Configuration/PathManagerTests.cs | 61 +++++++++++++++++++ test/DynamoCoreWpf3Tests/WorkspaceSaving.cs | 15 +++++ 4 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 test/DynamoCoreTests/Configuration/PathManagerTests.cs diff --git a/src/DynamoCore/Configuration/PathManager.cs b/src/DynamoCore/Configuration/PathManager.cs index 04c21722c45..8ff93dda382 100644 --- a/src/DynamoCore/Configuration/PathManager.cs +++ b/src/DynamoCore/Configuration/PathManager.cs @@ -869,6 +869,13 @@ private void BuildCommonDirectories() commonDataDir = GetCommonDataFolder(); defaultTemplatesDirectory = GetTemplateFolder(commonDataDir); + + // Seed the active templates directory with the default so it is never null. + // A preferred location supplied later through UpdatePreferenceItemPath may be + // rejected (unwritable %ProgramData%, unavailable network share, invalid path), + // and callers must still get a usable path rather than null. See DYN-10661. + templatesDirectory = defaultTemplatesDirectory; + rootDirectories = new List { userDataDir }; nodeDirectories = new HashSet diff --git a/src/DynamoCore/Models/DynamoModel.cs b/src/DynamoCore/Models/DynamoModel.cs index 81e01a6759f..6e7a4040b3f 100644 --- a/src/DynamoCore/Models/DynamoModel.cs +++ b/src/DynamoCore/Models/DynamoModel.cs @@ -1909,7 +1909,14 @@ private void InitializePreferenceLocations() UpdatePreferenceItemLocation(PreferenceItem.Backup, PreferenceSettings.BackupLocation); - UpdatePreferenceItemLocation(PreferenceItem.Templates, PreferenceSettings.TemplateFilePath); + if (!UpdatePreferenceItemLocation(PreferenceItem.Templates, PreferenceSettings.TemplateFilePath)) + { + // The preferred templates location could not be used, so the default stays + // in effect. This previously failed silently and only surfaced downstream as + // a null templates directory. See DYN-10661. + Logger?.Log("Could not use templates location '" + PreferenceSettings.TemplateFilePath + + "'. Falling back to '" + pathManager.TemplatesDirectory + "'.", LogLevel.File); + } } internal bool UpdatePreferenceItemLocation(PreferenceItem item, string newLocation) { diff --git a/test/DynamoCoreTests/Configuration/PathManagerTests.cs b/test/DynamoCoreTests/Configuration/PathManagerTests.cs new file mode 100644 index 00000000000..20788721881 --- /dev/null +++ b/test/DynamoCoreTests/Configuration/PathManagerTests.cs @@ -0,0 +1,61 @@ +using System.IO; +using Dynamo.Core; +using NUnit.Framework; + +namespace Dynamo.Tests.Configuration +{ + [TestFixture] + class PathManagerTests : UnitTestBase + { + private static PathManager MakePathManager() + { + return new PathManager(new PathManagerParams + { + CorePath = Path.GetDirectoryName(typeof(PathManager).Assembly.Location) + }); + } + + /// + /// DYN-10661: TemplatesDirectory has no default value. It is only ever assigned by + /// UpdatePreferenceItemPath, so a PathManager that has not had that call succeed + /// reports a null templates directory even though DefaultTemplatesDirectory is valid. + /// + [Test] + [Category("UnitTests")] + public void WhenPathManagerIsConstructedThenTemplatesDirectoryIsNotNull() + { + var pathManager = MakePathManager(); + + Assert.That(pathManager.DefaultTemplatesDirectory, Is.Not.Null.And.Not.Empty, + "DefaultTemplatesDirectory is built by the constructor and should always be set."); + Assert.That(pathManager.TemplatesDirectory, Is.Not.Null.And.Not.Empty, + "TemplatesDirectory must never be null - callers use it in string and path operations."); + } + + /// + /// DYN-10661: when the preferred templates location cannot be created (locked-down + /// %ProgramData%, unavailable network share, invalid path), UpdatePreferenceItemPath + /// returns false without assigning templatesDirectory, permanently leaving the + /// property null for the rest of the session. It should fall back to the default. + /// + [Test] + [Category("UnitTests")] + public void WhenTemplateLocationCannotBeCreatedThenTemplatesDirectoryFallsBackToDefault() + { + var pathManager = MakePathManager(); + + // A file occupying the target path makes Directory.CreateDirectory throw + // IOException, which is how PathHelper.CreateFolderIfNotExist reports an + // unusable location without surfacing an error to the user. + var blockedTemplateLocation = Path.Combine(TempFolder, "blockedTemplates"); + File.WriteAllText(blockedTemplateLocation, string.Empty); + + var updated = pathManager.UpdatePreferenceItemPath( + PathManager.PreferenceItem.Templates, blockedTemplateLocation); + + Assert.That(updated, Is.False, "An unusable location should be rejected."); + Assert.That(pathManager.TemplatesDirectory, Is.Not.Null.And.Not.Empty, + "A rejected templates location must leave a usable fallback, not null."); + } + } +} diff --git a/test/DynamoCoreWpf3Tests/WorkspaceSaving.cs b/test/DynamoCoreWpf3Tests/WorkspaceSaving.cs index 9adcad1e2b4..26b974bb2e6 100644 --- a/test/DynamoCoreWpf3Tests/WorkspaceSaving.cs +++ b/test/DynamoCoreWpf3Tests/WorkspaceSaving.cs @@ -588,6 +588,21 @@ public void TemplateSavePathCheckBlocksTemplateRootAndChildren() Path.Combine(templateRoot, "fr-FR", "Template.dyn"), localizedTemplateDirectory)); } + /// + /// DYN-10661: Save/SaveAs threw ArgumentNullException when the templates directory + /// was null, because the check was a bare path.Contains(templatesDirectory). + /// The check must treat an unknown templates directory as "not a template path". + /// + [Test] + [Category("UnitTests")] + public void TemplateSavePathCheckDoesNotThrowWhenTemplateDirectoryIsUnknown() + { + var savePath = Path.Combine(TempFolder, "Workspace.dyn"); + + Assert.IsFalse(DynamoViewModel.IsPathInTemplateDirectoryTree(savePath, null)); + Assert.IsFalse(DynamoViewModel.IsPathInTemplateDirectoryTree(savePath, string.Empty)); + } + [Test] [Category("UnitTests")] public void TemplateSavePathCheckAllowsPathsOutsideTemplateRoot() From fd921c75d96c0a8a4646e3defa2fb760d097d0fd Mon Sep 17 00:00:00 2001 From: Jason Date: Tue, 28 Jul 2026 18:13:05 -0400 Subject: [PATCH 2/2] DYN-10661: assert the templates fallback is the default Address review feedback: the test name promised a fallback to the default templates directory but the assertion only checked for a non-empty value, so it would have passed for any other non-default path. Compare against DefaultTemplatesDirectory instead. Co-Authored-By: Claude Opus 5 (1M context) --- test/DynamoCoreTests/Configuration/PathManagerTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/DynamoCoreTests/Configuration/PathManagerTests.cs b/test/DynamoCoreTests/Configuration/PathManagerTests.cs index 20788721881..0d7d802c5f5 100644 --- a/test/DynamoCoreTests/Configuration/PathManagerTests.cs +++ b/test/DynamoCoreTests/Configuration/PathManagerTests.cs @@ -54,8 +54,8 @@ public void WhenTemplateLocationCannotBeCreatedThenTemplatesDirectoryFallsBackTo PathManager.PreferenceItem.Templates, blockedTemplateLocation); Assert.That(updated, Is.False, "An unusable location should be rejected."); - Assert.That(pathManager.TemplatesDirectory, Is.Not.Null.And.Not.Empty, - "A rejected templates location must leave a usable fallback, not null."); + Assert.That(pathManager.TemplatesDirectory, Is.EqualTo(pathManager.DefaultTemplatesDirectory), + "A rejected templates location must leave the default in effect, not null."); } } }