From: Tim Cross <email@example.com> To: Timothy <firstname.lastname@example.org> Cc: email@example.com Subject: Re: [PATCH] Accept more :tangle-mode specification forms Date: Mon, 22 Nov 2021 00:51:19 +1100 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> Timothy <firstname.lastname@example.org> writes: > Hi Tim, > >>> The parsing of “555” to the integer 555 is done by >>> org-babel-parse-header-arguments, and so can’t really be changed. >> >> I don't understand this. Why can't it be changed? > > Well, it can't be changed without changing > org-babel-parse-header-arguments, which is quite a major change and I > suspect may be a breaking change. > > Maybe it's fine? I just don't feel confident about this. > >> When I said disable base 10 values, what I meant was generate an error >> if the value looks like it is base 10 i.e. does not have a leading o or >> #o. This makes it clear it has to be specified as an octal value and >> will alert users to this fact. > > This is all well and good, but the point I'm making is that since due to > the current behaviour of org-babel-parse-header-arguments we can't > distinguish between :tangle-mode (identity #o555) and :tangle-mode 365 > putting this idea /into practice/ doesn't look like it will be easy. > I don't see the complication here. When you call org-babel-parse-header-arguments, it returns an alist of argument name and value. If you had a header like :tangle-mode 555, you get the value (:tangle-mode . 555), if you have :tangle-mode o555, you get (:tangle-mode . "o555"). My suggestion is simply to look at the value and if it is not a string, reject it outright. If it is a string, then match that string to the allowed formats (i.e. an octal value if it starts with "#0o" or "o", a symbolic value if it matches a regexp for symbolic representation and even possiblly your ls -l format if it matches that. This is not a change in org-babel-parse-arguments, but rather a change in how we interpret those arguments. > It's also worth noting that with regard to the recent changes that have > been made, this is not a new issue. Which I don't think is a valid argument for the status quo. Allowing base 10 mode specifications is just a bad idea. The fact it has been permitted in the past is likely just an oversight rather than intentional behaviour. Given the potential security issues involved here, I think we can legitimately obsolete such features (using acceptable change management approaches with suitable notification for users etc). > >>> I think the ls-style is quite valuable for two reasons. It’s >>> well-established thanks to ls, and is particularly good for >>> understanding permissions at a glance. >> >> I would agree it is an established way to display the permissions >> associated with a filesystem object, but disagree it is a well >> established way to set such values - I know of no other tool which uses >> this format. > > The driving motivation here is that while the tools which you mention do > not use this for setting permissions (e.g. chmod), they are only used > for /setting/ permissions. Since Org files aren't write-only, and > occasionally we go back and look at what we've written :P I think > allowing a format that is optimised for ease-of-reading instead of > ease-of-writing makes some sense. It is in this respect that I think > ls -l style is a good idea. > I understand your motivation. I dispute your claim it is an established and common convention. >> It is also not the same as the ls -l display (no object >> type indicator). The ls -l also displays sticky bit, uid and gid. Does >> your format also support setting these values (something which can be >> done with the octal or symbolic formats) i.e. support s, S, t and T for >> the 'executable' bit for user/group? > > Ah, I'm afraid that I'm not that up-together on sticky bits. I suspect > that it's not just the ls -l style that will need tweaking. I'll read up > on this in the next few days and update the ML. > This is very difficult Timothy. I really do appreciate all the work you have done here and your patience, which makes what I'm going to say all the more difficult. My big concern here is that you don't understand this area with sufficient depth of understanding to really appreciate some of the subtleties involved here. This makes me concerned that any implementation of a new way to specify mode settings may have some unexpected side effects or hidden issues which could be exploited in unexpected ways (worse case) or simply have unexpected bugs which result in mode settings which are not what the user intended. As someone who has used Unix since 1980 and Linux since 1995, I've experienced my share of issues relating to file modes and their interaction with different filesystems, NFS, different platforms, character sets, locales etc. Implementing a new approach as you are proposing is something I would be extremely wary about. If you do plan to go forward with this approach, please ensure you are very confident in your understanding of the sticky bit, setuid/setgid and associated interaction with execute etc. I suspect you do have the skills to implement correctly given sufficient time, testing and motivation. However, I am concerned your initial stab at this may be somewhat naive and requires more consideration. My main concern is that once you take all the subtleties into consideration, the complexity of your ls -l approach will exceed its utility and maintainability. I could also be completely wrong. >> Personally, I prefer the symbolic form as it is shorter and clear. I >> find the ls -l form too easy to get wrong (especially with getting the >> number of '-' correct). > > Isn't choice a great thing? :D In seriousness, this is exactly why I > think it's worth providing these options. At least thinking back to when > I started using Linux a few years ago I would have said the same thing > about the octal form, and was completely unfamiliar with chmod. I expect > different people to have different preferences with these three options, > but for one of them to be something most people are happy with. > Choice is good. However, as I mentioned ina previous post, the problem with choice is complexity and the biggest source of security problems is complexity. Keep in mind that what you do now is something which will need to be maintained by others in the future. It needs to be clear, concise and as simple as possible (but of course, no simpler!). >>> Tim suggested that invalid forms should cause tangling to fail, but I feel this >>> may be a bit much. Personally I’m inclined to either >>> • warn, and don’t touch the file permissions (this is what currently occurs) >>> • use a very conservative file permission (e.g. rw——-). >> >> I'm unsure on this. My concern is people may not notice the warning and >> then be surprised later. Given the potential security issues, a later >> surprise is something to be avoided even if it is inconvenient. With >> respect to the default action to take, I would suggest we also need to >> look at the default umask setting for the user and if that is more >> restrictive, use that rather than some value like rw------- For all we >> know, the user has set a specific umask for a valid reason and will be >> surprised if emacs just ignores that to do what it wants (another >> surprise to be avoided). > >> The user is not required to specify a mode. However, if they do and if >> we cannot interpret what they have specified without ambiguity, we >> should throw an error and cease processing. Making a guess as to what >> they intended in this situation is IMO a big mistake. > > I don't see how using the default permissions is a potential security > risk? Could it surprise the user, yes, but that seems unavoidable when > the user is doing something invalid. Consider this simple scenario. The tangled output is being written to a directory which requires specific permissions for security reasons - perhaps it is some type of ftp server where permissions will determine what can be seen by whom. The data might contain sensitive information. The user specifies what they thing is the correct mode specification, but the7y get it wrong. Org comes along and looks at what they specified and decides it cannot interpret what the user has specified, so applies a default (and potentially completely wrong) mode, exposing sensitive data to others who should not have had access. Use a default if the user does not specify a mode, but if they specify a mode and org cannot interpret what they specified, then do nothing - don't generate the tangled output at all. This is the only safe approach. > >> My only preference for "#o" over just "o" prefix is to clearly signal to >> the user that it is an octal value and avoid the situation where you >> might glance a value and see the leading 'o' as a '0', missing the point >> it is an octal value not a decimal one. However, this seems like a low >> risk, so I'm happy either way. > > Well, I've got "o" for now but that's not set in stone. With a lower > case "o" I feel the risk for confusion with "0" is quite low (besides > which a "0" prefix seems to be the C-style for octal anyway). > I am not overly concerned about this one. I merely mention the use of "#0" as a possible approach and why. If the preference is just to have 'o', that is fine with me. My main point is that when it comes to parsing the value, having "#o" as a prefix for octal values is possibly slightly easier wrt regexp specification than just 'o' as 'o' could also be the first character for a symbolic mode specification as in o+w.
next prev parent reply other threads:[~2021-11-21 14:31 UTC|newest] Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-30 18:14 Timothy 2021-10-01 1:24 ` Tom Gillespie 2021-10-01 6:59 ` Timothy 2021-10-01 8:00 ` Stefan Nobis 2021-10-01 10:05 ` Eric S Fraga 2021-10-01 10:29 ` tomas 2021-10-01 18:04 ` Tom Gillespie 2021-10-01 18:14 ` Timothy 2021-10-01 8:39 ` Christian Moe 2021-10-05 14:45 ` Timothy 2021-10-05 15:54 ` email@example.com 2021-10-05 16:13 ` Timothy 2021-10-05 16:06 ` tomas 2021-10-06 11:59 ` Max Nikulin 2021-11-18 10:20 ` Timothy 2021-11-18 17:22 ` Timothy 2021-11-18 23:33 ` Tom Gillespie 2021-11-19 16:31 ` Tim Cross 2021-11-19 18:10 ` tomas 2021-11-20 4:31 ` Greg Minshall 2021-11-20 8:08 ` Timothy 2021-11-20 12:25 ` tomas 2021-11-20 14:50 ` Timothy 2021-11-20 16:09 ` tomas 2021-11-20 21:32 ` Tim Cross 2021-11-21 4:08 ` Greg Minshall 2021-11-21 4:27 ` Timothy 2021-11-21 5:11 ` Greg Minshall 2021-11-20 19:49 ` Tim Cross 2021-11-21 4:02 ` Timothy 2021-11-21 13:51 ` Tim Cross [this message] 2021-11-21 14:33 ` Timothy 2021-11-29 18:57 ` Timothy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: https://www.orgmode.org/ * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH] Accept more :tangle-mode specification forms' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this inbox: https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).